KKcorps commented on PR #20111:
URL: https://github.com/apache/pinot/pull/20111#issuecomment-6555551924
This comment comes from a model of upsert thread orders that StarTree runs
on upsert PRs.
**This closes the race it targets, and the model found nothing new. A start
that waited on the lineage lock while the other run completed now fails, so
brokers no longer serve both outputs.**
None of the model's known bad orders start two replaces over one segment, so
none of them change. A 71 minute search, 5,467 runs from 120 starting states,
found the second COMPLETED entry only without this PR. It needs a second run
over the same segments while the first still runs. On an upsert table the rows
then show twice only when both outputs are past metadataTTL, since the server
marks those rows valid without taking their keys.
**The order it still allows.** If the second run starts while the first is
IN_PROGRESS, forceCleanup deletes the first output. When that output's add
moved a key's previous location, the row a consuming segment's revert restores,
onto itself, a later reset drops the key. Retention does the same to an output
stuck IN_PROGRESS, so this is not a regression.
```mermaid
sequenceDiagram
participant B as Run B
participant A as Run A
participant C as Controller
participant S as Server
participant N as Consumer N3
B->>B: reads N0 before run A uploads
A->>C: startReplaceSegments(N0 to R2)
A->>S: upload R2
S->>S: add R2, A's previous location moves from N0 to R2
B->>C: startReplaceSegments(N0 to R1), forceCleanup
C->>C: revert run A's IN_PROGRESS entry
C->>S: delete R2
N->>S: reset, revert finds R2 gone, drops A
N->>S: replays A's latest row with no base row
Note over S: A is 1 where reading every row once gives 3
```
<details><summary>Model history</summary>
```
('append', 'A', (0,), 0, {'v': 1}, False)
('append', 'A', (1,), 1, {'v': 1}, False)
('append', 'D', (0,), 2, {'v': 1}, False)
('assign_consumer', "N0'")
('acquire', "N0'")
('consume', "N0'", 0)
('consume', "N0'", 1)
('consume', "N0'", 2)
('commit_local', "N0'", 2)
('step', 'replace', "N0'", 'start_replace')
('step', 'replace', "N0'", 'add_committed_row:A@0')
('step', 'replace', "N0'", 'add_committed_row:A@1')
('step', 'replace', "N0'", 'add_committed_row:D@2')
('step', 'replace', "N0'", 'begin_leftover')
('step', 'replace', "N0'", 'untrack_and_offload')
('append', 'B', (5,), 3, {'v': 1}, False)
('assign_consumer', "N1'")
('acquire', "N1'")
('consume', "N1'", 3)
('commit_local', "N1'", 3)
('step', 'replace', "N1'", 'start_replace')
('step', 'replace', "N1'", 'add_committed_row:B@3')
('step', 'replace', "N1'", 'begin_leftover')
('step', 'replace', "N1'", 'untrack_and_offload')
('append', 'C', (6,), 4, {'v': 1}, False)
('assign_consumer', "N2'")
('acquire', "N2'")
('consume', "N2'", 4)
('commit_local', "N2'", 4)
('step', 'replace', "N2'", 'start_replace')
('step', 'replace', "N2'", 'add_committed_row:C@4')
('step', 'replace', "N2'", 'begin_leftover')
('step', 'replace', "N2'", 'untrack_and_offload')
('append', 'A', (7,), 5, {'v': 1}, False)
('assign_consumer', "N3'")
('acquire', "N3'")
('consume', "N3'", 5)
('sweep_metadata_ttl',)
# run B is planned and reads N0 before run A uploads
('plan_replace_run', ['N0'], 'R1', 'SNAPSHOT')
('refresh', ['N0'], 'R2', 0)
('step', 'refresh', 'R2', 'start_add')
('step', 'refresh', 'R2', 'add_row:A')
('step', 'refresh', 'R2', 'add_row:D')
('step', 'refresh', 'R2', 'register')
# R2's add moved A's previous location onto R2's copy
('check_replace_run', 'R1', True)
('lock_replace_run', 'R1', False, True)
# forceCleanup reverted run A's IN_PROGRESS entry and deletes R2
('step', 'delete_extra', 'R2', 'begin_delete')
('step', 'delete_extra', 'R2', 'skip_out_of_ttl')
('step', 'delete_extra', 'R2', 'untrack')
('offload', "N3'")
('step', 'offload', "N3'", 'stop')
('step', 'offload', "N3'", 'close_consumer')
('step', 'offload', "N3'", 'begin_remove')
('step', 'offload', "N3'", 'walk:A')
('step', 'offload', "N3'", 'finish_revert')
('step', 'offload', "N3'", 'release_semaphore')
('step', 'offload', "N3'", 'untrack')
# the revert found R2 gone and dropped A, so the replay of A@7 has no base
('assign_consumer', "N3'r1")
('acquire', "N3'r1")
('consume', "N3'r1", 5)
```
</details>
The model covers one replica of one partition, and it runs the lineage
replace only on metadataTTL tables.
<!-- upsert-model-check head=e5f97ae1603663d28e62c0d042939bfbb66ec1bb -->
--
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]
