AkshayUpadhyay2411 opened a new issue, #19831: URL: https://github.com/apache/pinot/issues/19831
On a FULL upsert REALTIME table with `metadataTTL`, `enableSnapshot` and `enablePreload`, a server restart can leave one replica returning duplicate rows for the same primary key, while the other replicas return one row. The same query returns different results depending on the replica it is routed to. Reloading the segments on that server fixes it until the next time it happens. Seen on release-1.3.0 after a rolling restart, and reproduced on master. Logs of the affected server, before and after the restart: ``` Could not get segmentLock to take snapshot for segment: <segment>, skipping Missed taking snapshot for 5 immutable segments Failed to find validDocIds snapshot to add segment: <segment> out of TTL, treating all docs as valid ``` ### Cause `BasePartitionUpsertMetadataManager.doTakeSnapshot()` takes each segment's lock with `tryLock()` and skips the segment when the lock is held, e.g. by a segment reload or replace. The skipped segment keeps a stale validDocIds snapshot on disk, or none if it never had one, and stays in `_updatedSegmentsSinceLastSnapshot`. The TTL watermark is persisted at the end of the pass anyway. After a restart, the watermark is loaded from disk, and the segment is out of TTL against it. `doAddSegment()` / `doPreloadSegment()` then take the out-of-TTL path (`skipAddSegmentOutOfTTL()` / `skipPreloadSegmentOutOfTTL()`), which trusts the snapshot as is, or treats all docs as valid when there is no snapshot. The superseded rows of that segment become visible again, on that replica only. Not persisting the watermark when a segment is skipped is not enough on its own: during a restart, the watermark in memory is bumped after every segment that is added (#19512), and with preload the segments with snapshots are loaded first, so later segments can still be out of TTL in memory. ### To reproduce 1. FULL upsert REALTIME table with `metadataTTL` set, `enableSnapshot` and `enablePreload` true. 2. Ingest a set of primary keys, then ingest updates for all of them with comparison values more than `metadataTTL` later, while the segment holding the old rows is being reloaded, so the snapshot pass skips it. 3. Restart the server. 4. `SELECT id, COUNT(*) FROM t GROUP BY id HAVING COUNT(*) > 1` returns rows when routed to that server. ### Expected behavior One row per primary key on every replica after a restart, killed or graceful, without a reload. ### Proposed fix In `BasePartitionUpsertMetadataManager`, without changing `stop()`: - Cap the persisted TTL watermark so that segments still tracked as updated after the snapshot pass stay within TTL. The watermark in memory, used to expire primary keys, is not changed. - Only add or preload a segment as out of TTL if it is also out of TTL against the last persisted watermark, so the result does not depend on the order segments are loaded in. Unit tests for the restart cases (load orders, with and without preload, missing and stale snapshots) and an integration test that restarts a server after a missed snapshot. PR to follow. Related: #9062, #10928, #12232 (validDocIds snapshots), #18860 (reload from snapshot for TTL tables), #19512 (TTL watermark bump after segment add). -- 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]
