druid icon indicating copy to clipboard operation
druid copied to clipboard

Add sequential sketch merging to MSQ

Open adarshsanjeev opened this issue 3 years ago • 0 comments

Current Implementation

In the current MSQ implementation, each worker maintains a ClusterByStatisticsCollector. If we are clustering by time, one sketch is maintained for each time chunk. During the merge, we sent the entire ClusterByStatisticsCollector from the worker to the controller. These are merged together, downsampling if memory taken is too much and partitions are generated from this.

Potential Improvements

Taking all worker sketches into controller memory is an unneeded step, which leads to downsampling of sketches and generating partitions outside the targeted weight. Since we have a primary partitioning on time, we only need to maintain the sketch from all workers for a particular time partition, generate the partition and remove the sketches. This is the sequential sketch merging approach.

Since this increases time taken however, this has been moved to a separate WorkerSketchFetcher. This will use an executor service to avoid blocking the main controller thread. This will not remove the current functionality, it will only uses sequential merging under cases that it is likely to provide greater benefits. For small sketches, it continues to fetch the entire sketch for the speed.

Key changes

  • Add a new phase to controller: MERGING_STATISTICS and necessary transitions
  • Change worker to now send a WorkerReport to the controller, which contains information necessary to fetch sketches later.
  • Change the sketch fetching a pull from the controller instead of a push from the worker.
  • Add WorkerSketchFetcher to fetch sketches in the background.

Key changed/added classes in this PR
  • ClusterByStatisticsCollector
  • WorkerClient
  • ControllerClient
  • StageDefinition
  • WorkerSketchFetcher
  • ControllerStageTracker
  • ClusterByStatisticsWorkerReport

This PR has:

  • [ ] been self-reviewed.
    • [ ] using the concurrency checklist (Remove this item if the PR doesn't have any relation to concurrency.)
  • [ ] added documentation for new or modified features or behaviors.
  • [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • [ ] added or updated version, license, or notice information in licenses.yaml
  • [ ] 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.
  • [ ] added integration tests.
  • [ ] been tested in a test Druid cluster.

adarshsanjeev avatar Oct 11 '22 02:10 adarshsanjeev