gianm edited a comment on pull request #10892: URL: https://github.com/apache/druid/pull/10892#issuecomment-779963284
The change looks like a good idea to me. How about: - changing all call sites - removing `findEntry` and the PartitionHolder `asImmutable` method - adding a performance test that verifies that CachingClusteredClient is able to do its work in a reasonable amount of time even when there are some crazy number of segments in a time chunk, like 100,000? (That ought to be enough for anybody.) The "reasonable" amount of time might need to be somewhat high to prevent flaky tests from being an issue. Maybe 5–15 seconds. In reality, we'd want this to happen much faster than that, but we don't want to cause flaky tests. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
