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

Computer: fallback on transport for get_minimum_job_poll_interval default #5457

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Mar 21, 2022

Fixes #5455

The get_minimum_job_poll_interval returns a default when no value was
ever explicitly defined for the Computer instance. This is currently
provided by the PROPERTY_MINIMUM_SCHEDULER_POLL_INTERVAL__DEFAULT
attribute which is set to 10 seconds. This is a reasonable default for
most cases, but for the localhost it is probably excessive and can lead
to users waiting for a long time when running singular jobs on the
localhost.

The minimum job poll interval is not strictly a property of the
transport but in this case it is the transport that defines what happens
to be a sensible default. Therefore we still choose to add a new
attribute DEFAULT_MINIMUM_JOB_POLL_INTERVAL to Transport which the
Computer can use as a default if none is specified on the instance.
The Transport base class sets it to 10, which is backwards compatible
with the current behavior, and the LocalTransport then overrides it to
0.1. We choose a small finite number such that the waiting time in
most cases should not really be noticeable without running the risk that
the CPUs will really spin like crazy for jobs that run slightly longer.

@sphuber sphuber force-pushed the fix/5455/localhost-minimum-job-poll-interval-default branch from ba7c0a7 to 070d892 Compare March 21, 2022 22:48
Copy link
Member

@ramirezfranciscof ramirezfranciscof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

… default

The `get_minimum_job_poll_interval` returns a default when no value was
ever explicitly defined for the `Computer` instance. This is currently
provided by the `PROPERTY_MINIMUM_SCHEDULER_POLL_INTERVAL__DEFAULT`
attribute which is set to 10 seconds. This is a reasonable default for
most cases, but for the localhost it is probably excessive and can lead
to users waiting for a long time when running singular jobs on the
localhost.

The minimum job poll interval is not strictly a property of the
transport but in this case it is the transport that defines what happens
to be a sensible default. Therefore we still choose to add a new
attribute `DEFAULT_MINIMUM_JOB_POLL_INTERVAL` to `Transport` which the
`Computer` can use as a default if none is specified on the instance.
The `Transport` base class sets it to 10, which is backwards compatible
with the current behavior, and the `LocalTransport` then overrides it to
`0.1`. We choose a small finite number such that the waiting time in
most cases should not really be noticeable without running the risk that
the CPUs will really spin like crazy for jobs that run slightly longer.
@sphuber sphuber force-pushed the fix/5455/localhost-minimum-job-poll-interval-default branch from 070d892 to 88c7c60 Compare April 21, 2022 11:25
@sphuber sphuber merged commit f58d711 into aiidateam:develop Apr 21, 2022
@sphuber sphuber deleted the fix/5455/localhost-minimum-job-poll-interval-default branch April 21, 2022 11:50
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

Successfully merging this pull request may close these issues.

Default minimum_job_poll_interval is really defined by Computer and not the Transport class
2 participants