mluvin-stripe opened a new issue, #19707:
URL: https://github.com/apache/pinot/issues/19707

   ## Problem
   https://github.com/apache/pinot/pull/18282 mentions in its description 
   > `lineageEntryCleanupRetentionPeriod`: controls how long stale 
`IN_PROGRESS`/`REVERTED` lineage entries wait before cleanup (default: 1d)
   
   and in a [code 
comment](https://github.com/apache/pinot/pull/18282/changes#diff-b40104bb94a315fb720efb79ca588742275fba0c3a9d3f646e417f2522a00737R138-R140)
   ```
   /**
      * Returns the retention period before stale IN_PROGRESS or REVERTED 
lineage entries and their destination segments
      * are cleaned up. Consumers of this config (e.g. the lineage manager) 
treat a null or unparseable value as a
      * 1 day default.
   ``` 
   
   but the retention period is not respected for `REVERTED` lineage entries -- 
they're instead cleaned up immediately here 
https://github.com/apache/pinot/blame/04cca1a58dc23155e1b1fd355fe5da0b633622d3/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/lineage/DefaultLineageManager.java#L110-L112
   
   **Is this a bug, or is the code comment (and PR description) incorrect as to 
what the intended behavior is?**
   
   ## Impact of the bug
   Our segment metadata push job retries 3 times to push segments -- each time 
with the same segment names, and in each case a segment lineage entry is 
created. **So if a task run fails then succeeds, it leaves two entries with the 
same segment names in the entries’ `segmentsTo` field with different statuses 
`REVERTED` and `COMPLETED`**. 
   
   Later when the `RetentionManager` runs (in the case when we observed this 
bug, it happened to be immediately after `COMPLETED` segment push), it sees the 
segment names referenced in the `REVERTED` entry’s `segmentsTo` exist in the 
table’s IdealState (because there was a successful push later) and **ends up 
deleting them**.
   
   ## Potential solutions
   First, clarify what the intended behavior is – if 
`lineageEntryCleanupRetentionPeriod` is supposed to be respected for `REVERTED` 
entries, make the fix.
   
   Separately, we should consider not marking `REVERTED` entries’ segments for 
delete if there’s a later `COMPLETED` entry with any of the same `segmentsTo` 
names. **This fix should guarantee we never delete "live" segments.**
   


-- 
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]

Reply via email to