jadami10 commented on PR #9244: URL: https://github.com/apache/pinot/pull/9244#issuecomment-1224930629
> The purpose of this status checker is to consume as many events as possible before enabling query execution. We shouldn't care if the messages are fresh or not. For example, let's say the freshness threshold is 15s and assume there's a huge spike in the topic in last 15s and there are thousands of events available in the topic. The ingestion time for these events is less than the threshold so the status checker announces the consumption has caught up while there are a lot of messages available in the topic to be consumed. Others can chime in here as well about their SLAs/use cases. But for our use case, we promise an internal freshness SLA, not anything else based on number of events. So if there's thousands of events that are < N seconds old, we are fine starting the system up. I assumed this was generic enough of a streaming system SLA to provide as a feature in Pinot. > I agree it's easier to develop and test this way, but in the long run duplicated code has a high maintenance cost. Let's iron out the issues and consolidate the two classes into one. There's no longer any duplicated code as it's in a parent class, so I don't see the value in merging the classes when they'll be controlled by separate config values anyway. -- 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
