Skip to content

[9.5] fix(es): key existing documents on _id instead of _source.id (#4523) - #4573

Merged
Jan-Kazlouski-elastic merged 1 commit into
9.5from
backport/9.5/pr-4523
Oct 7, 2026
Merged

Jan-Kazlouski-elastic merged 1 commit into
9.5from
backport/9.5/pr-4523

Conversation

@github-actions

@github-actions github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Backports the following commits to 9.5:

## Closes #2776

When the sink loads the documents already in an index,
`yield_existing_documents_metadata` keyed each one on `_source.id` and
only fell back to `_id` when `id` was missing. Everything that consumes
that map treats the key as the Elasticsearch `_id`: `get_docs` and
`get_access_control_docs` look up each incoming doc by the `_id` they
are about to write, and `enqueue_docs_to_delete` issues bulk deletes by
`_id`.

The connector itself always writes `id` equal to `_id`, so the two only
diverge when something else changes `id`, for example an ingest
pipeline. When they do, no incoming doc matches an existing one: every
doc is counted as a create, `skip_unchanged_documents` never skips
anything, and the deletes at the end of a full sync target `_id` values
that don't exist, so docs removed from the source are left in the index.

This change keys the map on `_id` and drops `id` from the scan's
`_source`, since it is no longer read. The `id` field the connector
writes into documents is unchanged.

This changes behaviour for indices already written where `id` differs
from `_id`. The first full or access control sync after this change will
delete documents that were orphaned by the old behaviour, so document
counts may drop. Existing docs will be counted as updates rather than
creates, and unchanged docs will be skipped when
`skip_unchanged_documents` is enabled. Indices where `id` equals `_id`,
or where `id` is absent, are unaffected.

Tests: `test_management_client.py` gets a case where `_source.id`
differs from `_id` and asserts the yielded keys are the `_id`s.
`test_sink.py` gets a case that runs `get_docs` through the real
`yield_existing_documents_metadata` (with `async_scan` mocked) and
asserts the matching doc is queued as an update and the stale doc is
deleted by its `_id`. Both fail without the fix.

## Checklists

#### Pre-Review Checklist
- [x] this PR does NOT contain credentials of any kind, such as API keys
or username/passwords (double check `config.yml.example`)
- [x] this PR has a meaningful title
- [x] this PR links to all relevant github issues that it fixes or
partially addresses
- [x] Covered the changes with automated tests
- [ ] Tested the changes locally (ran `make lint` and `make test`; not
tested against a live cluster)
- [ ] Added a label for each target release version (example: `v7.13.2`,
`v7.14.0`, `v8.0.0`)
- [ ] For bugfixes: backport safely to all minor branches still
receiving patch releases

## Release Note

Fixes syncs failing to delete stale documents, and treating existing
documents as new, when an ingest pipeline changes a document's `id`
field so that it no longer matches its `_id`.

Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Jan-Kazlouski-elastic <jan.kazlouski@elastic.co>
@github-actions
github-actions Bot requested a review from a team as a code owner October 6, 2026 16:47
@Jan-Kazlouski-elastic
Jan-Kazlouski-elastic merged commit 81d6a5a into 9.5 Oct 7, 2026
2 checks passed
@Jan-Kazlouski-elastic
Jan-Kazlouski-elastic deleted the backport/9.5/pr-4523 branch October 7, 2026 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants