navina commented on PR #8564: URL: https://github.com/apache/pinot/pull/8564#issuecomment-1104519349
> I think we should strongly consider updating the interface so that the close() can do whatever the underlying stream wants to do. If that means checkpointing in the middle of an idle stream, so be it. You can add another method to the interface whose default implementation is to call the prev close. You should not need to modify any other stream that way @mcvsubbu This actually feels like an undiscovered bug for idle streams / errored streams due to the `LLRealtimeSegmentDataManager` constantly closes and recreates the consumer. Having said, Jackie made a good point about not overloading the semantics of close. If `LLRealtimeSegmentDataManager` needs full control over the consumer, then it is better to be explicit about invoking the `checkpoint` method. That is the contract made with `PartitionGroupConsumer`. I really don't think an interface change is needed for a bug fix. It will be a non-trivial change. -- 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
