deepthi912 opened a new pull request, #19125:
URL: https://github.com/apache/pinot/pull/19125
## Summary
`PartialUpsertTableRebalanceIntegrationTest#testReload` (and its downstream
RocksDB subclass) can flake with `expected [ONLINE] but found [OFFLINE]` at
`verifySegmentAssignment` line 304. The failure is a state-check-too-early
issue in the test, not a product bug.
## Why
`waitForReloadToComplete` polls the reload job status API, which only
records per-segment reload completion. For a partial-upsert reload in
`PROTECTED` consistency mode, the CONSUMING segment is force-committed, the
controller atomically flips it CONSUMING → ONLINE and creates a new CONSUMING
segment. But:
1. The reload job's `totalSegmentCount` is a snapshot at job start — it
doesn't include the new CONSUMING segments created by force-commit.
2. `waitForAllDocsLoaded` uses broker `SELECT COUNT(*)` which routes via
ExternalView; it cannot detect that an IdealState replica flipped OFFLINE while
another still serves.
3. If a server-side error (e.g., consumer init failure on the new segment)
fires `postStopConsumedMsg`,
`PinotLLCRealtimeSegmentManager.segmentStoppedConsuming` marks the segment
OFFLINE in IdealState; `RealtimeSegmentValidationManager` then creates a repair
segment. IdealState now has an older segment with `seq < maxSeq` in OFFLINE
state — which is exactly what the assertion sees.
None of the existing waits look at IdealState directly.
## Changes (test-only, no product code)
**File:**
`pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PartialUpsertTableRebalanceIntegrationTest.java`
1. New `waitForClusterStateSettled(table, expectedSegments, stableSamples,
timeoutMs)` that requires all of:
- IdealState has `expectedSegments` entries.
- No segment has any instance in OFFLINE state.
- `ExternalView.mapFields.equals(IdealState.mapFields)`.
- Holds for `stableSamples` consecutive polls (5 samples at 200ms → ~1s
of stability).
2. `testReload` calls it between `waitForAllDocsLoaded` and
`verifyIdealState`.
3. `verifySegmentAssignment` assertions now include segment name, instance,
`maxSequenceNumber`, and the full assignment map. If a real server-side stall
causes OFFLINE (not a race), the CI log will surface which segment on which
instance — not a bare `expected [ONLINE] but found [OFFLINE]`.
## Test plan
- [x] `./mvnw spotless:apply checkstyle:check -pl pinot-integration-tests` —
clean
- [x] `./mvnw -pl pinot-integration-tests -am test
-Dtest=PartialUpsertTableRebalanceIntegrationTest#testReload` — passes (23s)
- [ ] CI: rerun the flaky suite; expect no regression and cleaner failure
messages if a real stall surfaces
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
