KKcorps commented on PR #20097:
URL: https://github.com/apache/pinot/pull/20097#issuecomment-6518291669
This comment comes from a model of upsert thread orders that StarTree runs
on upsert PRs.
**A server cannot tell a force commit from a normal rollover, so every move
this change adds or removes already happens today, and the model found no bad
order that only one side reaches.**
A 77 minute search, 1,444 runs over 15 table shapes, compared the old and
new force commit rules on the server that receives the partition.
**The order both allow, which later steps now widen.** On a partial upsert
table the new server's consumer waits at most for the segment before it,
because the all segments loaded check passed when the table first loaded there.
A replayed row can then merge before the segment holding that key's current row
has loaded. The key then serves a wrong value, and every replica does if this
one commits the segment. With this PR a later step replays the whole successor
in that window, where today's second force commit leaves only rows that arrive
after the move. Partitions first picked after the recalculation now shrink it
instead.
```mermaid
sequenceDiagram
participant R as TableRebalancer
participant O as Staying replica
participant H as New server, Helix threads
participant C as New server, consumer N3
R->>O: step 1 force commits N2
Note over O: B arrives at offset 3 and lands in N3
R->>H: step 2 moves the partition with no commit
H->>H: N0 and N2 ONLINE, N1 still loading
H->>C: N3 CONSUMING from the end offset of N2
C->>C: N2 is registered, so it starts
C->>C: replays B at offset 3, no base row, B is 1
H->>H: N1 loads, its older row for B loses
Note over C: B stays 1 where reading every row once gives 2
```
<details><summary>Model history</summary>
```
('append', 'A', (0,), 0, {'v': 1}, False)
('append', 'B', (0,), 1, {'v': 1}, False)
('commit_remote', 0, 0)
('commit_remote', 1, 1)
('append', 'A', (0,), 2, {'v': 1}, False)
# step 1 force commits N2 at the stream end
('commit_remote', 2, 2)
('append', 'B', (0,), 3, {'v': 1}, False)
# step 2 moves the partition here with no second force commit
('join_fresh_replica', True)
('step', 'add_immutable', 'N0', 'start_add')
('step', 'add_immutable', 'N0', 'add_row:A@0')
('step', 'add_immutable', 'N0', 'register')
('step', 'add_immutable', 'N2', 'start_add')
('step', 'add_immutable', 'N2', 'add_row:A@2')
('step', 'add_immutable', 'N2', 'register')
# N1 still loading, N3' only waits for N2
('acquire', "N3'")
('consume', "N3'", 3)
('step', 'add_immutable', 'N1', 'start_add')
('step', 'add_immutable', 'N1', 'add_row:B@1')
('step', 'add_immutable', 'N1', 'register')
```
</details>
The model covers one replica and no dedup, so it skips the replica that
drops the partition.
<!-- upsert-model-check head=e666346ec02255bd3c8d4c6ad0e466db9bf65c3c -->
--
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]
