suvodeep-pyne opened a new pull request, #20157:
URL: https://github.com/apache/pinot/pull/20157
## Problem
Since #19909, every segment-level validation run resets the ERROR replicas
of committed segments on pauseless dedup and partial-upsert tables. A reset
reloads the segment, and the replica then serves queries again.
On these tables, reloading is not always safe. After the server's dedup /
partial-upsert consume gate
(`RealtimeTableDataManager._isTableReadyToConsumeData`) latches, a failed
segment N does not stop the server from consuming N+1, N+2, … without N's keys.
Those later segments can then hold rows that the healthy replica drops as
duplicates, or partial-upsert rows merged against missing previous values.
While N is in ERROR, strict replica-group routing keeps that server out of the
query path. The reset loads N, so routing includes the server again, and its
wrong rows become visible.
Before #19909, pauseless tables returned early here unless repair was
explicitly allowed.
## Change
On a pauseless table where `allowRepairOfCommittingSegments` is false,
`repairSegmentsInErrorState` now skips every reset of ERROR replicas, not only
the resets that re-consume. That is a dedup or partial-upsert table without an
opt-in. The skip covers DONE and UPLOADED segments, and COMMITTING segments
whose ideal state is already ONLINE.
- A skipped segment counts in
`PAUSELESS_SEGMENTS_IN_UNRECOVERABLE_ERROR_COUNT`. A WARN names both opt-ins.
- Operators opt in for one run with the task property
`repairErrorSegmentsForPartialUpsertOrDedup=true`, or for the table with
`disasterRecoveryMode=ALWAYS`. Both already exist.
- From #19909, resets still go only to ERROR replicas
(`resetErrorReplicas`). Healthy replicas are never reset.
- Non-pauseless tables, and pauseless tables without dedup or partial
upsert, behave as on master.
```mermaid
flowchart TD
A[Segment with an ERROR replica] --> B{COMMITTING and all replicas in
ERROR?}
B -- yes --> R[Re-ingest only if repair is allowed]
B -- no --> G{Pauseless table and repair not allowed?<br/>dedup or partial
upsert, no opt-in}
G -- yes --> U[Skip the reset; count as unrecoverable; WARN naming the
opt-ins]
G -- no --> C{Still CONSUMING in the ideal state?}
C -- yes --> W[Reset only if all replicas are in ERROR; otherwise wait for
the commit]
C -- no --> L[Reset the ERROR replicas; they reload the segment]
```
## Where to start
`PinotLLCRealtimeSegmentManager.repairSegmentsInErrorState`: the new gate
sits right after the re-ingestion branch, and it replaces the IN_PROGRESS-only
gate that was there before.
## Cost and known gaps
- **The stuck server that #19909 fixed comes back.** The consume gate
latches per table on each server. If a committed segment fails before the
latch, ingestion stops for every partition of that table on that server. If two
servers each fail a segment of a different partition, strict routing can leave
no server to query. Both conditions last until an operator opts in. The
unrecoverable gauge and ingestion lag show it.
- This PR does not cover these other paths:
- `segmentStoppedConsuming` always calls `resetPartition`. After a
consumer init fails, a peer that commits the segment within the 30 s callback
delay still gets the segment reloaded without an opt-in.
- Retention can delete a segment while it is in ERROR. That lifts the
routing exclusion while later segments on that server may hold wrong rows.
- Non-pauseless dedup and partial-upsert tables have the same hazard under
`controller.segment.error.autoReset`.
- Both opt-ins also allow re-ingestion and re-consuming resets, as before.
The root fix is to stop a server from consuming past a failed segment on
these tables. That needs its own design and follows separately.
## Testing
- `PinotLLCRealtimeSegmentManagerTest` (80/80 pass):
- Gated dedup and partial-upsert tables, with DONE, UPLOADED,
COMMITTING-but-ONLINE, IN_PROGRESS and all-ERROR COMMITTING segments: no reset,
and 7 unrecoverable segments. These cases fail on master, which resets the
committed segments.
- The same tables opted in, by task property or by
`disasterRecoveryMode=ALWAYS`: only ERROR replicas are reset, and the gauge is
0.
- Pauseless full-upsert and non-pauseless dedup tables: resets go out as
before.
- `PauselessDedupErrorReplicaRepairTest`: a real cluster, where one server
fails the first consuming segment of each partition.
- Without an opt-in, three validation runs 10 s apart leave both ERROR
replicas unchanged, and the unrecoverable gauge stays at 2. On master, the
first run resets them.
- A run with `repairErrorSegmentsForPartialUpsertOrDedup=true` then
repairs them. The broker routes to both servers again, and queries return the
deduplicated count.
- Checkstyle, spotless and license checks pass. The changed lines add no
compiler warnings.
Label: `bugfix`
--
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]
