You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "kfaraz (via GitHub)" <gi...@apache.org> on 2023/05/12 09:03:01 UTC

[GitHub] [druid] kfaraz opened a new pull request, #14269: Update default values in CoordinatorDynamicConfig

kfaraz opened a new pull request, #14269:
URL: https://github.com/apache/druid/pull/14269

   ## Changes
   
   The defaults of the following config values in the `CoordinatorDynamicConfig` are being updated.
   
   #### 1. `maxSegmentsInNodeLoadingQueue = 500` (previous = 100)
   
   Rationale: With round-robin segment assignment now being the default assignment technique, the Coordinator can assign a large number of under-replicated/unavailable segments very quickly. Before round-robin, a large queue size would cause the Coordinato to get stuck in `RunRules` duty due to very slow strategy-based cost computations.
   
   #### 2. `replicationThrottleLimit = 500` (previous = 10)
   Rationale: Along with the reasoning given for `maxSegmentsInNodeLoadingQueue`, a very low `replicationThrottleLimit` can cause clusters to be very slow in getting to full replication, even when there are loading threads sitting idle.
   
   Note: It is okay to keep this value equal to `maxSegmentsInNodeLoadingQueue`. Even with equal values, load queues will not get filled up with just replicas, and segments that are completely unavailable will still get a fair chance. This is because while MSINLQ applies to a single server, `replicationThrottleLimit` applies to each tier.
   
   #### 3. `maxSegmentsToMove = 100` (previous = 5)
   
   Rationale: A very low value of this config (say 5) turns out to be very ineffective in balancing especially if there are a large number of segments in a cluster and/or a large skew between usages of two historical servers.
   On the other hand, a very large value can cause excessive moves every minute, which might have the following disadvantages:
   - Load of moving segments competing with load of unavailable/under-replicated segments
   - Unnecessary network costs due to constant download and delete of segments
   
   These defaults will be revisited after #13197 is merged.
   
   ## Testing
   
   These values have been tried on different production cluster sizes, and have been found to give satisfactory results.
   
   #### Release note
   Update default values of the following coordinator dynamic configs:
   - `maxSegmentsInNodeLoadingQueue = 500`
   - `maxSegmentsToMove = 100`
   - `replicationThrottleLimit = 500`
   
   <hr>
   
   <!-- Check the items by putting "x" in the brackets for the done things. Not all of these items apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items from the checklist below are strictly necessary, but it would be very helpful if you at least self-review the PR. -->
   
   This PR has:
   
   - [ ] been self-reviewed.
      - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
   - [ ] added documentation for new or modified features or behaviors.
   - [ ] a release note entry in the PR description.
   - [ ] 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](https://github.com/apache/druid/blob/master/dev/license.md)
   - [ ] 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](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   - [ ] added integration tests.
   - [ ] been tested in a test Druid cluster.
   


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] ektravel commented on a diff in pull request #14269: Update default values in CoordinatorDynamicConfig

Posted by "ektravel (via GitHub)" <gi...@apache.org>.
ektravel commented on code in PR #14269:
URL: https://github.com/apache/druid/pull/14269#discussion_r1197924642


##########
docs/configuration/index.md:
##########
@@ -951,16 +951,16 @@ Issuing a GET request at the same URL will return the spec that is currently in
 |`millisToWaitBeforeDeleting`|How long does the Coordinator need to be a leader before it can start marking overshadowed segments as unused in metadata storage.|900000 (15 mins)|
 |`mergeBytesLimit`|The maximum total uncompressed size in bytes of segments to merge.|524288000L|
 |`mergeSegmentsLimit`|The maximum number of segments that can be in a single [append task](../ingestion/tasks.md).|100|
-|`maxSegmentsToMove`|The maximum number of segments that can be moved at any given time.|5|
+|`maxSegmentsToMove`|The maximum number of segments that can be moved at any given time.|100|
 |`useBatchedSegmentSampler`|Deprecated. Boolean flag for whether or not we should use the Reservoir Sampling with a reservoir of size k instead of fixed size 1 to pick segments to move. This option can be enabled to speed up the sampling of segments to be balanced, especially if there is a large number of segments in the cluster or if there are too many segments to move.|true|
 |`percentOfSegmentsToConsiderPerMove`|Deprecated. This will eventually be phased out by the batched segment sampler. You can enable the batched segment sampler now by setting the dynamic Coordinator config, `useBatchedSegmentSampler`, to `true`. Note that if you choose to enable the batched segment sampler, `percentOfSegmentsToConsiderPerMove` will no longer have any effect on balancing. If `useBatchedSegmentSampler == false`, this config defines the percentage of the total number of segments in the cluster that are considered every time a segment needs to be selected for a move. Druid orders servers by available capacity ascending (the least available capacity first) and then iterates over the servers. For each server, Druid iterates over the segments on the server, considering them for moving. The default config of 100% means that every segment on every server is a candidate to be moved. This should make sense for most small to medium-sized clusters. However, an admin may find it 
 preferable to drop this value lower if they don't think that it is worthwhile to consider every single segment in the cluster each time it is looking for a segment to move.|100|
 |`replicantLifetime`|The maximum number of Coordinator runs for a segment to be replicated before we start alerting.|15|
-|`replicationThrottleLimit`|The maximum number of segments that can be replicated at one time.|10|
+|`replicationThrottleLimit`|The maximum number of segments that can be in the replication queue of a historical tier at any given time.|500|
 |`balancerComputeThreads`|Thread pool size for computing moving cost of segments in segment balancing. Consider increasing this if you have a lot of segments and moving segments starts to get stuck.|1|
 |`emitBalancingStats`|Boolean flag for whether or not we should emit balancing stats. This is an expensive operation.|false|
 |`killDataSourceWhitelist`|List of specific data sources for which kill tasks are sent if property `druid.coordinator.kill.on` is true. This can be a list of comma-separated data source names or a JSON array.|none|
 |`killPendingSegmentsSkipList`|List of data sources for which pendingSegments are _NOT_ cleaned up if property `druid.coordinator.kill.pendingSegments.on` is true. This can be a list of comma-separated data sources or a JSON array.|none|
-|`maxSegmentsInNodeLoadingQueue`|The maximum number of segments that could be queued for loading to any given server. This parameter could be used to speed up segments loading process, especially if there are "slow" nodes in the cluster (with low loading speed) or if too much segments scheduled to be replicated to some particular node (faster loading could be preferred to better segments distribution). Desired value depends on segments loading speed, acceptable replication time and number of nodes. Value 1000 could be a start point for a rather big cluster. Default value is 100. |100|
+|`maxSegmentsInNodeLoadingQueue`|The maximum number of segments that could be queued for loading to any given server. This parameter could be used to speed up segments loading process, especially if there are "slow" nodes in the cluster (with low loading speed) or if too much segments scheduled to be replicated to some particular node (faster loading could be preferred to better segments distribution). Desired value depends on segments loading speed, acceptable replication time and number of nodes. Value 1000 could be a start point for a rather big cluster. |500|

Review Comment:
   ```suggestion
   |`maxSegmentsInNodeLoadingQueue`|The maximum number of segments allowed in a loading queue for any given server. Use this parameter to load the segments faster&mdash;for example, if the cluster contains slow-loading nodes, or if there are too many segments to be replicated to a particular node (when faster loading is preferred to better segments distribution). Desired value depends on the loading speed of segments, acceptable replication time, and number of nodes. Value 1000 is a good starting point for a big cluster. |500|
   ```



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] AmatyaAvadhanula commented on pull request #14269: Update default values in CoordinatorDynamicConfig

Posted by "AmatyaAvadhanula (via GitHub)" <gi...@apache.org>.
AmatyaAvadhanula commented on PR #14269:
URL: https://github.com/apache/druid/pull/14269#issuecomment-1545427014

   Changes may be needed in `coordinator-dynamic-config.tsx` as well.


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] kfaraz commented on pull request #14269: Update default values in CoordinatorDynamicConfig

Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on PR #14269:
URL: https://github.com/apache/druid/pull/14269#issuecomment-1554079956

   Thanks for the review, @ektravel ! I have incorporated your feedback.


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] kfaraz merged pull request #14269: Update default values in CoordinatorDynamicConfig

Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz merged PR #14269:
URL: https://github.com/apache/druid/pull/14269


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org