a2l007 edited a comment on pull request #10284: URL: https://github.com/apache/druid/pull/10284#issuecomment-674244483
Lucas > If the ServerHolder objects were passed in an indeterminate order, I would agree. But since they are not, I think that doing this could start to work against what I am trying to accomplish here. As an admin, I have determined that we are wasting our time bothering to consider segments once we have considered a certain amount of them. Because, after that point, the probability of choosing one of these segments is so low that I have deemed it to be not worth the resources used to consider it. My implementation says, if under normal balancing conditions (no limit), that all the segments beyond some point (probably) wouldn't be chosen anyways, let's just bail out early. Your suggestion here somewhat changes the fundamentals of the balancing strategy by now considering segments that would under normal balancing conditions, have had near zero chance of being chosen to now having a chance of being chosen, that I as an admin, have determined is likely enough to spend the resources to fi nd out. And worse yet, those segments that may be chosen from ServerHolders at the end of the list are from Servers that I would probably prefer to be receiving segments rather than giving them up (in terms of consumption, not locality/performance), unless my cluster is very well balanced by consumption already. a2l007 > Thanks for clarifying. My suggestion was basically ignoring the fact that the serverholder list is sorted by available space. In case we don't proceed to go the percentage route in this PR, it may be helpful to provide some recommendation in the docs on what might be a good value to set this to percentage-wise. Finally circling back to this now that I have time... I decided to just update the doc for now. I am still trying to determine if the overhead to do percentages would require compute overhead that defeats the purpose of this. Looking at the code as I type this update.. `ServerHolder` contains an `ImmutableDruidServer` which has `int` instance variable `numSegments`. So I guess we could quickly get numSegments since it was essentially pre-computed for us. I worried that we'd have to stream the segments map to get the number on the fly. So all in, we'd iterate the ServerHolders first and get the number of segments and then calculate our `%` and use the same code with this new limiter. I do really like this as it is more flexible for long term durability. The raw number approach may require TLC when segment and server counts change significantly. I think we could chalk up the overhead to calculate the number of segments to consider as negligible. Say a cluster has `200 servers with 1 0k segments per server`. If we do `25%` we will add a loop that iterates a 200 item list and do a little math that will reduce the number of segment considerations from 2MM to 500k. We'd also skip the calculation if the value is 100% so still no added overhead for our default value users. ---------------------------------------------------------------- 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: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
