Skip to content

PHOENIX-7961 Secondary index diverges from data table after TTL expiry on partial-touch upserts - #2602

Open
sanjeet006py wants to merge 1 commit into
apache:5.3from
sanjeet006py:fix-index-data-table-sync-5.3
Open

PHOENIX-7961 Secondary index diverges from data table after TTL expiry on partial-touch upserts#2602
sanjeet006py wants to merge 1 commit into
apache:5.3from
sanjeet006py:fix-index-data-table-sync-5.3

Conversation

@sanjeet006py

Copy link
Copy Markdown
Contributor

Cherry-pick of #2574 onto the 5.3 branch.

What changes were proposed in this pull request?

This PR makes Phoenix's internal current-row read during secondary-index maintenance TTL-mask exactly like an ordinary client read, so a data table and its secondary index stay consistent after TTL expiry.

Index maintenance in IndexRegionObserver.preBatchMutateWithExceptions reads the current on-disk row via getCurrentRowStates to rebuild the correct index entry. That read was opened directly through region.getScanner(scan), which bypasses the postScannerOpen coprocessor hook — the only place a scan is normally wrapped in TTLRegionScanner. As a result the internal read was not TTL-masked, so on a "partial touch" upsert (an UPSERT that does not re-write any index-referenced column) the index was rebuilt from logically-expired-but-not-yet-compacted cells, while the data table correctly expired them on read.

Changes:

  1. Client side — ScanUtil.annotateMutationWithLiteralTTL (called from MutationState) threads onto each mutation exactly what the client read path sets as scan attributes: the empty-column CF/CQ (unconditionally for any literal-TTL table/view), a view's compiled literal TTL as the _TTL attribute (base tables rely on the CF-descriptor TTL), and IS_STRICT_TTL=false only when the table/view is non-strict.
  2. Server side — new ServerScanUtil (package org.apache.phoenix.coprocessor, module phoenix-core-server) reproduces the client read path server-side: setInternalScanAttributes / setInternalScanAttributesForPaging set the same empty-column/TTL/strict/paging attributes, and openRegionScanner wraps the scan in TTLRegionScanner + PagingRegionScanner exactly as postScannerOpen does. IndexRegionObserver captures the client-threaded attributes into the batch context (extractLiteralTTLForInternalScan) and passes them into getCurrentRowStates.
  3. Anchor the masking clock at batchTimestamp — batchTimestamp is computed before the current-row read, and the internal scan sets scan.setTimeRange(0, batchTimestamp) so TTLRegionScanner's "current time" equals the exact timestamp at which the index is rebuilt, closing a sub-millisecond boundary window between the scan-open wall clock and batchTimestamp. The half-open range drops nothing current, since every pre-existing locked-row cell was written at ts < batchTimestamp.
  4. Companion correctness fix — moving getBatchTimestamp earlier means a registered set could be structurally mutated outside its synchronized block; batchesWithLastTimestamp now registers a defensive TreeSet snapshot to avoid a ConcurrentModificationException, staying conservative for the sleep/timestamp invariant (at most an extra sleep, never a missed one).

Production files touched: IndexRegionObserver, ScanUtil, new ServerScanUtil, MutationState, and a doc-comment update in MetaDataClient.

Why are the changes needed?

This is a silent data-integrity/correctness bug. On a table or view with a literal TTL and a secondary index, after TTL expiry the secondary index can return a column value the data table no longer returns — the two diverge with no error raised. The divergence surfaces after logical TTL expiry and before major compaction physically purges the expired cells.

The root cause is that the internal current-row read bypassed TTLRegionScanner, so index rebuild saw expired cells the data-side read masks. The affected paths are: secondary indexes (global covered, global uncovered, and immutable indexes whose data/index storage schemes differ) and the no-index current-row reads on a literal-TTL table (atomic / ON DUPLICATE KEY upserts, returnResult upserts, and row deletes). Conditional-TTL, non-TTL, and non-strict-TTL tables are unaffected — masking is a no-op there and the read is byte-identical to before.

Does this PR introduce any user-facing change?

Yes — a bug fix (behavior change) relative to 5.3 and released versions, for tables/views with a literal TTL and a secondary index.

  • Before: on a partial-touch upsert near/after the TTL boundary, the secondary index could return a value that the data table no longer returned (index resurrected a logically-expired value); data and index disagreed.
  • After: the internal current-row read is TTL-masked identically to a client read and anchored at batchTimestamp, so the index is rebuilt from the same masked state the data table exposes, and the two stay consistent.

No API, syntax, or configuration change. No new config flag is introduced.

How was this patch tested?

New integration test IndexDataTableSyncIT (parameterized over column-encoded on/off), covering:

  • base-table covered-column re-sync;
  • uncovered-index indexed-column re-sync;
  • view covered-column re-sync;
  • non-strict table is not masked;
  • no-index atomic (ON DUPLICATE KEY) upsert masks an expired row;
  • UPSERT ... SELECT and UPSERT ... VALUES touches near the TTL boundary keep data and index consistent;
  • an immutable index with a differing storage scheme;
  • a concurrency regression (testConcurrentMajorCompactionDuringIndexWriteKeepsDataAndIndexConsistent) where a data-table major compaction runs during a slow index write — after the current-row read and after the data locks are released, but before the index write completes — and data/index must remain consistent.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Anthropic Claude Opus 4.8)

@sanjeet006py sanjeet006py added the backport Backport PR label Aug 18, 2026
@sanjeet006py
sanjeet006py force-pushed the fix-index-data-table-sync-5.3 branch from 1affc10 to e599099 Compare August 18, 2026 06:18
@sanjeet006py

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Backport PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant