gortiz commented on PR #12343:
URL: https://github.com/apache/pinot/pull/12343#issuecomment-1920959709
I've changed the code to call
`RealtimeTableDataManager.onConsumingToDropped()` from
`SegmentOnlineOfflineStateModelFactory.onBecomeOfflineFromConsuming`, which
seems closer to what Jackie suggested.
Anyway, I think this part of the code is a bit messy. It goes against
structured concurrency (or in general any structured technique) to have this
topology of calls:
```mermaid
stateDiagram-v2
SegmentOnlineOfflineStateModelFactory --> InstanceDataManager : Start (1)
SegmentOnlineOfflineStateModelFactory --> InstanceDataManager : Stop (3)
SegmentOnlineOfflineStateModelFactory --> IngestionDelayTracker : Stop
(4)
InstanceDataManager --> IngestionDelayTracker : New values(2)
```
Instead we should look for a pattern where `IngestionDelayTracker` is
something only known by `InstanceDataManager`. Then
`SegmentOnlineOfflineStateModelFactory` will always and only notify
`InstanceDataManager` and it is the responsability of `InstanceDataManager` to
start and stop `IngestionDelayTracker`. Like:
```mermaid
stateDiagram-v2
SegmentOnlineOfflineStateModelFactory --> InstanceDataManager : Start (1)
SegmentOnlineOfflineStateModelFactory --> InstanceDataManager : Stop (3)
InstanceDataManager --> IngestionDelayTracker : New values(2)
InstanceDataManager --> IngestionDelayTracker : Stop (4)
```
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
