batch-scheduler: fix the number of cross-vm links computation #367
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a bug in the logic to calculate the number of cross-VM links.
Before, we calculated the number of cross-VM links as the product of the number of executors per VM.
This is only true if we have only 2 VMs.
Take for example the two following scheduling decision:
[1, 1, 1, 1]
and[2, 2]
.By doing the product, the former has a number of cross-VMs of 1, and the latter of 4, which means that the planner would prefer the former over the latter.
In fact, the right algorithm to calculate the number of cross-VM links is to, for each host, add all non-local executors in different hosts, and times it for the number of multiple hosts. And divide by two at the end to remove duplicates.
The updated algorithm gives a number of cross-VM links of 6 and 4 respectively, making the planner prefer the latter over the former.