capistrant commented on PR #18844: URL: https://github.com/apache/druid/pull/18844#issuecomment-3747123669
> > Thanks for addressing the feedback proactively, @capistrant ! The new code structure with the fingerprint mapper looks much better, and has made a couple of things apparent. > > The major open items that need to be addressed are: > > > > * Update the cache when upserting a fingerprint in the metadata store. > > * Persist the target compaction state only when committing segments. > > * Move cleanup duty to Overlord > > * Rename new table and columns > > Addressed 1 and 3 > > 2 - I have commented about the **why** behind this choice, but need to consider more the problems/risks you call out cuz they seem legit > > 4 - I still plan to do last for purposes of the diff. Now that most all of the major things are sorted, I will stage commits for it locally For 2, I ended up going with the idea of a state being `pending` when it it inserted in to the database by the job queue. The overlord cleanup duty uses a secondary cleanup mechanism for pending compaction states, allowing operators to be more aggressive about cleaning up the unused states while not risking a compaction state being nuked before a long running compaction task completes. When inserting new segments, the coordinator will flip any associated compaction states from pending to "active" (pending=false) if needed. -- 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]
