You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/06/30 21:43:50 UTC
[GitHub] [druid] a2l007 opened a new pull request #10105: Clarify change in behavior for druid.server.maxSize
a2l007 opened a new pull request #10105:
URL: https://github.com/apache/druid/pull/10105
With #10070 , `druid.server.maxSize` is no longer just a soft limit on the total segment size that can be assigned to a historical. If this property is set to the default value of 0 for a historical, coordinator will no longer assign segments to it.
This doc change attempts to clarify this behavior.
Long term, it would be useful to identify a more realistic default value for this property instead of zero.
----------------------------------------------------------------
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.
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] gianm commented on pull request #10105: Clarify change in behavior for druid.server.maxSize
Posted by GitBox <gi...@apache.org>.
gianm commented on pull request #10105:
URL: https://github.com/apache/druid/pull/10105#issuecomment-652136504
What was the behavior change that happened in #10070?
I looked at that patch, and I read the diff in the docs here, and it isn't clear to me what changed.
----------------------------------------------------------------
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.
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] gianm commented on pull request #10105: Clarify change in behavior for druid.server.maxSize
Posted by GitBox <gi...@apache.org>.
gianm commented on pull request #10105:
URL: https://github.com/apache/druid/pull/10105#issuecomment-652149734
I see. That older behavior sounds like a bug that got fixed, but perhaps people were relying on the buggy behavior. So I agree it would be good to call it out. Thanks for explaining it.
----------------------------------------------------------------
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.
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] clintropolis commented on a change in pull request #10105: Clarify change in behavior for druid.server.maxSize
Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #10105:
URL: https://github.com/apache/druid/pull/10105#discussion_r448066624
##########
File path: docs/configuration/index.md
##########
@@ -1370,7 +1370,7 @@ These Historical configurations can be defined in the `historical/runtime.proper
|Property|Description|Default|
|--------|-----------|-------|
-|`druid.server.maxSize`|The maximum number of bytes-worth of segments that the process wants assigned to it. This is not a limit that Historical processes actually enforces, just a value published to the Coordinator process so it can plan accordingly.|0|
+|`druid.server.maxSize`|The maximum number of bytes-worth of segments that the process wants assigned to it. The Coordinator process will attempt to assign segments to a Historical process only if this property is greater than the total size of segments served by it. Since this property defines the upper limit on the total segment size that can be assigned to a Historical, it needs to be set to a value greater than zero.|0|
Review comment:
Should this mention that this size should be equal(ish) to the sum of all segment cache location sizes? Ideally someday we can drop this property and just compute it from the segment cache config, but .. that day is not today, so maybe it is worth including.
----------------------------------------------------------------
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.
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] clintropolis merged pull request #10105: Clarify change in behavior for druid.server.maxSize
Posted by GitBox <gi...@apache.org>.
clintropolis merged pull request #10105:
URL: https://github.com/apache/druid/pull/10105
----------------------------------------------------------------
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.
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] a2l007 edited a comment on pull request #10105: Clarify change in behavior for druid.server.maxSize
Posted by GitBox <gi...@apache.org>.
a2l007 edited a comment on pull request #10105:
URL: https://github.com/apache/druid/pull/10105#issuecomment-652146731
----------------------------------------------------------------
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.
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] gianm commented on pull request #10105: Clarify change in behavior for druid.server.maxSize
Posted by GitBox <gi...@apache.org>.
gianm commented on pull request #10105:
URL: https://github.com/apache/druid/pull/10105#issuecomment-652148057
> @gianm Prior to that patch, coordinator could assign segments to historicals even if the balancer couldn't find a valid move for a segment. This means that even if `druid.server.maxSize` isn't optimally configured for a historical, the coordinator could still assign segments to it.
What do you mean by "could assign segments even if the balancer couldn't find a valid move"? Like, if all historicals were full (no room in `druid.server.maxSize`), it would assign to one of the full ones anyway?
----------------------------------------------------------------
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.
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] a2l007 commented on pull request #10105: Clarify change in behavior for druid.server.maxSize
Posted by GitBox <gi...@apache.org>.
a2l007 commented on pull request #10105:
URL: https://github.com/apache/druid/pull/10105#issuecomment-652146731
@gianm Prior to that patch, coordinator could assign segments to historicals even if the balancer couldn't find a valid move for a segment. This means that even if `druid.server.maxSize` isn't optimally configured, the coordinator could still assign segments to it.
However, with the patch the coordinator avoids segment assignment if it cannot find valid moves. This means that existing clusters configured with incorrect `druid.server.maxSize` property could see coordinators not assigning segments even though the historicals have enough capacity. So I feel the docs needs to emphasize the importance of this property a bit more given the behavioral change.
----------------------------------------------------------------
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.
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] gianm edited a comment on pull request #10105: Clarify change in behavior for druid.server.maxSize
Posted by GitBox <gi...@apache.org>.
gianm edited a comment on pull request #10105:
URL: https://github.com/apache/druid/pull/10105#issuecomment-652136504
What was the behavior change that happened in #10070?
I looked at that patch's description, and I read the diff in the docs here, and it isn't clear to me what changed.
----------------------------------------------------------------
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.
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] a2l007 commented on pull request #10105: Clarify change in behavior for druid.server.maxSize
Posted by GitBox <gi...@apache.org>.
a2l007 commented on pull request #10105:
URL: https://github.com/apache/druid/pull/10105#issuecomment-652149255
> > @gianm Prior to that patch, coordinator could assign segments to historicals even if the balancer couldn't find a valid move for a segment. This means that even if `druid.server.maxSize` isn't optimally configured for a historical, the coordinator could still assign segments to it.
>
> What do you mean by "could assign segments even if the balancer couldn't find a valid move"? Like, if all historicals were full (no room in `druid.server.maxSize`), it would assign to one of the full ones anyway?
Yes I was referring to that behavior.
----------------------------------------------------------------
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.
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