KKcorps commented on PR #12250:
URL: https://github.com/apache/pinot/pull/12250#issuecomment-6286929746
**This clears none of the model's known bad orders and adds one, where a
partial-upsert table merges rows twice after a disable and enable.**
None of the 45 histories behind the model's known bad orders changed. A
62-minute search, 1,664 runs over 6 table shapes, broke 204 times only with
this change, each time on a key left pointing at an offloaded consuming segment.
**The order it adds.** While the table is disabled, `removeSegment()`
returns early, so a consuming segment's keys keep pointing at it after it goes
OFFLINE. On enable, Helix creates that segment again from its start offset.
Each key's last row ties its own old copy, so `doUpdateRecord` merges the two
and an INCREMENT column counts that row twice, for every key the segment held.
If the old segment was already destroyed, the merge reads freed buffers,
because a consuming segment takes no read lock.
```mermaid
sequenceDiagram
participant C as Controller
participant H as Helix thread
participant M as Metadata manager
participant N as New consuming segment
Note over M: A points at the consuming segment's row o1, value 2
C->>H: disable the table
H->>M: offload, removeSegment()
M-->>H: ideal state disabled, return early
Note over M: A still points at the old segment's row o1
C->>H: enable the table
H->>N: OFFLINE to CONSUMING, start at o0
N->>M: o0 is older than the pointer, no merge
N->>M: o1 ties the pointer, merge with old row o1
Note over M: A is 3, but the stream only adds up to 2
```
<details><summary>Model history</summary>
```
('append', 'A', (1,), 0, {'v': 1}, False)
('append', 'A', (2,), 1, {'v': 1}, False)
('assign_consumer', "N0'")
('acquire', "N0'")
('consume', "N0'", 0)
('consume', "N0'", 1)
('disable_table',)
('offload', "N0'")
('step', 'offload', "N0'", 'stop')
('step', 'offload', "N0'", 'close_consumer')
('step', 'offload', "N0'", 'begin_remove')
('step', 'offload', "N0'", 'release_semaphore')
('step', 'offload', "N0'", 'untrack')
('enable_table',)
('assign_consumer', "N0'r1")
('acquire', "N0'r1")
('consume', "N0'r1", 0)
('consume', "N0'r1", 1)
```
</details>
The model covers one replica and one partition, and it applies this change
to today's master.
<!-- upsert-model-check head=b9e7c2c042175798da332b04f90395b99e665612 -->
--
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]
