capistrant commented on PR #18844: URL: https://github.com/apache/druid/pull/18844#issuecomment-3708651869
@kfaraz I pushed up changes addressing the review you had to date. I ended up going with the idea of dropping support for fingerprinting from the CompactSegments duty. I also did my best to wrap my head around the segment metadata caching and tying in compaction state caching to that instead of having the caching as a one off in CompactionStateManager. now the manager just handles persists and then the lifecycle management of states in the database as they go unused and age out. I also took a crack at moving when the compaction states are persisted, per your suggestion. They no longer happen in the template code, but rather will happen when tasks are about to be ran. I have talked briefly with @clintropolis and I think he will be submitting a review soon with thoughts on changing the naming in the metastore to drop the "compaction" from the names since we are already do more than just compaction and are trending towards adding even more functionality than pure compaction. If you have any thoughts on table and column naming, I'd love to hear it. If we do refactor the naming, I think it will be up for debate how much we change naming in the app code in this PR vs another refactoring PR that focuses on making the naming more generic in the app code when it comes to compaction supervisors. -- 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]
