druid icon indicating copy to clipboard operation
druid copied to clipboard
verified publisher icon apache

Concurrent loading segment cache

Open GWphua opened this issue 4 months ago • 1 comments

Description

Concurrent loading of cached segments during startup

Previously, we are using 1 thread to run SegmentLocalCacheManager#getCachedSegments. We may take quite a bit of time if the number of segments is large.

We can do better by multithreading using loadOnBootstrapExec, and speed up segment retrieval.

Refactoring

  • Create segmentsToLoad array via extracted method: retrieveSegmentMetadataFiles.
  • Extract logic to load segments into addFilesToCachedSegments

Log Changes

  • Add stopwatch to report time taken to load cached segments.
  • Removed log "Loading segment cache file [%d/%d][%s]." to remove log clutter, stopwatch will help to handle this better. (It was annoying to navigate ~100k lines of this when looking through logs)

Release note

Speed up load of cached segments during Historical startup.

Key changed/added classes in this PR
  • SegmentLocalCacheManager
  • SegmentCacheManager + Relevant implementations.

This PR has:

  • [x] been self-reviewed.
    • [x] using the concurrency checklist (Remove this item if the PR doesn't have any relation to concurrency.)
  • [x] a release note entry in the PR description.
  • [x] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • [x] been tested in a test Druid cluster. (TBD)

GWphua avatar Sep 05 '25 10:09 GWphua

Hi @kfaraz, I have tried to changes, but the need to add a ExecutorService and use exec.submit() introduces another layer of indentation -- meaning the reviewing process will still be tedious.

I have created #18494 to help with reviewing this change, please take a look at that. I hope that after merging #18494, diff changes in this PR will be clearer. Thanks!

GWphua avatar Sep 07 '25 06:09 GWphua

Hi @kfaraz, hopefully the changes now are more reviewer-friendly. Can take a look again. Thanks!

GWphua avatar Dec 18 '25 03:12 GWphua

Thanks @kfaraz for the review! Comments are resolved, feel free to take a look again. :)

GWphua avatar Dec 18 '25 08:12 GWphua