Skip to content

PHOENIX-7894 Set EXPECTED_UPPER_REGION_KEY on local index scans - #2632

Open
apurtell wants to merge 3 commits into
apache:masterfrom
apurtell:PHOENIX-7894
Open

apurtell wants to merge 3 commits into
apache:masterfrom
apurtell:PHOENIX-7894

Conversation

@apurtell

@apurtell apurtell commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

The local index stale region check falls back to the scan stop row when EXPECTED_UPPER_REGION_KEY is absent. Reversed scans are swapped on the client, so the stop row is the region start key and every region looks stale.

Set the attribute to the region end key in setLocalIndexAttributes so the check is direction independent. When the check passes, setupLocalIndexScan must also accept the swapped boundaries. Take the prefix from the region start key in both directions. For a reversed scan, put the stop row suffix in the start row.

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

The local-index stale region check falls back to the scan stop row
when EXPECTED_UPPER_REGION_KEY is absent. Reversed scans are swapped
on the client, so the stop row is the region start key and every
region looks stale. Set the attribute to the region end key in
setLocalIndexAttributes so the check is direction-independent.

When the check passes, setupLocalIndexScan must also accept the
swapped boundaries. It took the key prefix from the start row, which
is the region end key for a reversed scan, so the scan returned no
rows. Take the prefix from the region start key in both directions.
For a reversed scan, put the stop row suffix in the start row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The older-client fallback still rejects client-swapped reversed scans as stale.

1 open finding
What changed in this PR

Adds direction-independent stale-region detection for local index scans.

Changes:

  • Records the expected region end key on local index scans.
  • Correctly rewrites reversed scan boundaries.
  • Adds boundary, split, merge, and open-ended-region tests.
File Description
ScanUtil.java Sets region-end metadata and handles reversed boundaries.
BaseScannerRegionObserver.java Exposes boundary checking for tests and updates documentation.
ScanUtilTest.java Tests local-index scan attributes.
LocalIndexScanBoundaryTest.java Tests server-side boundary detection and rewriting.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

TTL setup can overwrite the expected upper key, causing non-final local-index regions to appear stale.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation addresses the direction-dependent boundary bug and includes focused regression coverage.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants