Horton

Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY

Mihail Nikalayeu <michail.nikolaev@gmail.com>

View thread
From: Michail Nikolaev <michail.nikolaev@gmail.com>
To: Michael Paquier <michael@paquier.xyz>, Noah Misch <noah@leadboat.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2024-08-24T15:52:00Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Replace flaky CIC/RI isolation tests with a TAP test

  2. Disable recently added CIC/RI isolation tests

  3. Fix infer_arbiter_index for partitioned tables

  4. Stabilize tests some more

  5. Put back alternative-output expected files

  6. Remove doc and code comments about ON CONFLICT deficiencies

  7. Avoid use of NOTICE to wait for snapshot invalidation

  8. Fix ON CONFLICT with REINDEX CONCURRENTLY and partitions

  9. Fix ON CONFLICT ON CONSTRAINT during REINDEX CONCURRENTLY

  10. Fix new test for CATCACHE_FORCE_RELEASE builds

  11. Improve test case stability

  12. Fix infer_arbiter_index during concurrent index operations

  13. Doc: cover index CONCURRENTLY causing errors in INSERT ... ON CONFLICT.

  14. Fix infer_arbiter_indexes() to not assume resultRelation is 1.

  15. Revert temporal primary keys and foreign keys

Attachments

Hello, everyone!

This patch set addresses the issues discussed in this thread.

The main idea behind this fix is that it is safe to consider indisready
indexes alongside indisvalid indexes as arbiter indexes. However, it's
crucial that at least one fully valid index is present.

Why is it necessary to consider indisready during the planning phase?

The reason is that these indexes are required for correct processing during
the execution phase.
If "ready" indexes are skipped as arbiters by one transaction, they may
already have become "valid" for another concurrent transaction during its
planning phase.
As a result, both transactions could concurrently process the UPSERT
command with different sets of arbiters (while using the same set of
indexes for tuple insertion later).
This can lead to unexpected "duplicate key value violates unique
constraint" errors and deadlocks.

Is it safe to use a "ready" but not yet "valid" index as an arbiter?
Yes, as long as at least one "valid" index is also used as an arbiter.
The valid index ensures the correctness of the UPSERT logic, while the
"ready" index contains an equal or lesser number of tuples, making it safe
for speculative insertion.
In any case, the insert to that index will be processed during
ExecInsertIndexTuples one way or another (with applyNoDupErr or without).

Fix is divided into a few patches, each following this logic:

1) The first patch provides specs (and injection points) for the various
scenarios related to the issue.
2) The second patch introduces a straightforward change—adding indisready
indexes to arbiters alongside indisvalid. However, at least one indisvalid
is still required. This resolves simple cases involving REINDEX
CONCURRENTLY and CREATE INDEX CONCURRENTLY.
3) The third patch deals with named constraints. Instead of relying solely
on the index with the specified name, we attempt to find other indexes that
are equivalent in terms of being used as an arbiter.
4) This patch fixes a scenario involving partitioned tables. Special checks
are required for partitioned indexes, which may be processed by REINDEX
CONCURRENTLY.

Additionally, a patch with three extra TAP specifications for stress
testing is attached. This patch is not intended for commitment, so I
renamed the extension to prevent accidental application in some CI/DI jobs.

>
Also, it is possible to look at the patches on GitHub:
https://github.com/postgres/postgres/compare/master...michail-nikolaev:postgres:reindex_concurrently_with_upsert

Best regards,
Mikhail.