Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why is machine loading rounded? #484

Closed
expipiplus1 opened this issue Jun 5, 2017 · 7 comments
Closed

Why is machine loading rounded? #484

expipiplus1 opened this issue Jun 5, 2017 · 7 comments

Comments

@expipiplus1
Copy link
Contributor

https://github.com/NixOS/hydra/blob/master/src/hydra-queue-runner/dispatcher.cc#L131-L132

In the comparison for machine selection:

float ta = std::round(a.currentJobs / a.machine->speedFactor);
float tb = std::round(b.currentJobs / b.machine->speedFactor);

This doesn't seem necessary, and leads to incorrect results when the machines have a high speedFactor.

@expipiplus1
Copy link
Contributor Author

@domenkozar might have figured it out:

14:48 <domenkozar> jophish: yes, one side effect of the roundf is that same machine will be reused until speedfactor is reached
14:48 <domenkozar> so it might have a consequence of less S3 downloads

@edolstra
Copy link
Member

edolstra commented Jun 6, 2017

Yes, IIRC that was the reason.

(Looks like the C++ rewrite of build-remote got rid of the rounding BTW, which may be by accident.)

@expipiplus1
Copy link
Contributor Author

expipiplus1 commented Jun 6, 2017

Would it be more appropriate to divide with max jobs?

I've patched our local hydra to be a bit fairer in case anyone's interested :)

expipiplus1@73e835b

@someplaceguy
Copy link

I have simply removed the std::round on my local Hydra and it has been working great for me for a couple of months now. The workload is distributed much better now.

That said, my build machines don't do many S3 downloads as I compile everything from source, so I don't have to worry about that.

@Ericson2314
Copy link
Member

It is back to being floats!

@Ericson2314
Copy link
Member

Would it be more appropriate to divide with max jobs?

I've patched our local hydra to be a bit fairer in case anyone's interested :)

expipiplus1@73e835b

This is interesting and perhaps should be pursued anyways.

@someplaceguy
Copy link

It is back to being floats!

I don't understand. Wasn't this issue about the rounding, which causes the load to be distributed unevenly across the remote machines?

Based on the current code:

float ta = std::round(a.currentJobs / a.machine->speedFactor);
float tb = std::round(b.currentJobs / b.machine->speedFactor);

... isn't rounding still being performed, or what am I missing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants