Logical replication can lose an update after concurrent index invalidation - #436
Open
pg-hub-mirror[bot] wants to merge 1 commit into
Open
pg-hub-mirror[bot] wants to merge 1 commit into
pg-hub-mirror[bot] wants to merge 1 commit into
Conversation
When the apply worker searches the local relation by index, it can take the first match as the row only if the index is the relation's replica identity or primary key. Otherwise every match has to be compared with the search slot, which holds a complete row only under REPLICA IDENTITY FULL. Only the index OID was saved, so the scan worked this out a second time from the catalogs, and the two answers can differ. Apply holds only RowExclusiveLock, which doesn't conflict with DROP INDEX CONCURRENTLY or REINDEX CONCURRENTLY, so either can demote the chosen index in between. Nothing matches, so the change is silently dropped as an update_missing conflict, and assert-enabled builds fail. Fix this by recording the answer as idxisreplident in the relation map entry and passing the entry down to FindReplTupleInLocalRel(), so it is settled once. Oversight in 89e46da. Author: Mikhail Nikalayeu <mihailnikalayeu@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: vignesh C <vignesh21@gmail.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Discussion: https://postgr.es/m/CADzfLwUJovFcnknCC9wjZKECX9xecgnGzC2r2TMV8h4QDD_jwQ@mail.gmail.com Backpatch-through: 16, where it was introduced
Author
|
Earlier design discussion: Discussion #61 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pgsql-hackersCAA4eK1Jvw2h7ycGDTr6YXL22BUC-TC+dbM7z3Gs+B8MnHJqDug@mail.gmail.comPatch files:
On Mon, Sep 7, 2026 at 9:04 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
Fixed in the attached. Apart from this I changed multiple comments to
make those clear. One notable change is, I moved the newly added
boolean after localindexoid as it reads better there because then we
don't need to forward reference the fields. For back-branches, if it
needs to be moved to an earlier location then we can do that in those
versions but for HEAD and 19, the new location seems better.
Also, shall we keep just one test, say Drop Index Concurrently instead
of two as both tests do the same thing in a slightly different way? I
have not done that but if you agree please update the patch
accordingly.
With Regards,
Amit Kapila.