KKcorps commented on PR #19998:
URL: https://github.com/apache/pinot/pull/19998#issuecomment-6412035147
This comes from a model of upsert thread orders that StarTree runs on upsert
PRs, and it checked only the TTL watermark cap.
**Caching each segment's max closes the window from the earlier comment at
6004060, but a much narrower path to the same duplicate is left.**
I replayed the model's 45 known bad orders with and without the change, and
none changed. An hour of search over 8 table shapes found nothing new, and a
search started at a fresh marker found only this order.
**The order that is left.** `markSegmentUpdated` publishes
`TTL_WATERMARK_NOT_SET` before it reads the max, so a snapshot round that reads
the marker in between persists no watermark. An old segment N0 that comes back
ONLINE before the next round is then added in full. The window is one metadata
read wide, so it is much rarer than the destroy was, and today's code reaches
the same duplicate when N0 itself misses a round.
```mermaid
sequenceDiagram
participant R as Reload N1
participant C as Consumer N3
participant M as Upsert manager
participant H as Helix N0
R->>M: replaceSegment(N1) adds its rows
R->>M: putIfAbsent(new N1, TTL_WATERMARK_NOT_SET)
C->>M: getWatermarkToPersist reads NOT_SET for the new N1
M->>M: persist TTL_WATERMARK_NOT_SET
R->>M: read N1's max and replace NOT_SET
C->>M: removeExpiredPrimaryKeys drops A, its row A@6 in N2 stays valid
H->>M: N0 OFFLINE then ONLINE, addSegment(N0)
M->>M: no skip, so every N0 doc is compared
Note over M: A@0 finds no key, so A@0 and A@6 are both valid
```
<details><summary>Model history</summary>
```
('append', 'A', (0,), 0, {'v': 1}, False)
('append', 'C', (0,), 1, {'v': 1}, False)
('assign_consumer', "N0'")
('acquire', "N0'")
('consume', "N0'", 0)
('consume', "N0'", 1)
('commit_local', "N0'", 1)
('step', 'replace', "N0'", 'start_replace')
('step', 'replace', "N0'", 'read_marked_max')
('step', 'replace', "N0'", 'add_committed_row:A@0')
('step', 'replace', "N0'", 'add_committed_row:C@1')
('step', 'replace', "N0'", 'begin_leftover')
('step', 'replace', "N0'", 'untrack_and_offload')
('append', 'A', (4,), 2, {'v': 1}, False)
('assign_consumer', "N1'")
('acquire', "N1'")
('consume', "N1'", 2)
('commit_local', "N1'", 2)
('step', 'replace', "N1'", 'start_replace')
('step', 'replace', "N1'", 'read_marked_max')
('step', 'replace', "N1'", 'add_committed_row:A@2')
('step', 'replace', "N1'", 'begin_leftover')
('step', 'replace', "N1'", 'untrack_and_offload')
('append', 'B', (5,), 3, {'v': 1}, False)
('append', 'A', (6,), 4, {'v': 1}, False)
('append', 'D', (9,), 5, {'v': 1}, False)
('assign_consumer', "N2'")
('acquire', "N2'")
('consume', "N2'", 3)
('consume', "N2'", 4)
('consume', "N2'", 5)
('commit_local', "N2'", 5)
('step', 'replace', "N2'", 'start_replace')
('step', 'replace', "N2'", 'read_marked_max')
('step', 'replace', "N2'", 'add_committed_row:B@3')
('step', 'replace', "N2'", 'add_committed_row:A@4')
('step', 'replace', "N2'", 'add_committed_row:D@5')
('step', 'replace', "N2'", 'begin_leftover')
('step', 'replace', "N2'", 'untrack_and_offload')
('reload', 'N1')
('step', 'reload', 'N1', 'start_replace')
('assign_consumer', "N3'")
('acquire', "N3'")
('step_watermark_cap',)
('step', 'reload', 'N1', 'read_marked_max')
('step', 'reload', 'N1', 'add_row:A@2')
('step', 'reload', 'N1', 'begin_leftover')
('step', 'reload', 'N1', 'untrack')
('sweep_expired_keys', ['A', 'C', 'B'])
('offline_immutable', 'N0')
('step', 'offline', 'N0', 'begin_remove')
('step', 'offline', 'N0', 'skip_out_of_ttl')
('step', 'offline', 'N0', 'untrack')
('add_immutable', 'N0')
('step', 'add_immutable', 'N0', 'start_add')
('step', 'add_immutable', 'N0', 'read_marked_max')
('step', 'add_immutable', 'N0', 'add_row:A@0')
('step', 'add_immutable', 'N0', 'add_row:C@1')
('step', 'add_immutable', 'N0', 'register')
```
</details>
Reading the max under the read lock when the loop meets an unset value, as
6004060 did, would close it for every live segment.
The model covers one replica and one partition, and not the mapped metadata
sidecar.
<!-- upsert-model-check head=9e251211d1bfed8914fbf81c1d534bbc3bdfc64e -->
--
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]
