sanjeet006py opened a new pull request, #2602:
URL: https://github.com/apache/phoenix/pull/2602
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://phoenix.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
PHOENIX-XXXX Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes, and
prefix it with the JIRA id, e.g. 'PHOENIX-XXXX Your PR title ...'.
6. If possible, provide a concise example to reproduce the issue for a
faster review.
-->
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)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
