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

Tidy up htex worker pool start time recording #3112

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

benclifford
Copy link
Collaborator

The start time is needed as part of upcoming work on walltime-based worker draining (so this PR makes it into an instance attribute)...

... and it can be made more accurate than the previous implementation by measuring it earlier (most especially, prior to invoking the often expensive probe_addresses call)

This PR rephrases a nearby log line which uses the word "starting" despite being not in the start() procedure of the manager, to reduce ambiguity when reading the code vs logs.

Changed Behaviour

Users who pay attention to this log line:

    logger.info("process_worker_pool ran for {} seconds".format(delta))

might notice a slight increase in the seconds count for "the same" duration runs.

Type of change

  • Update to human readable text: Documentation/error messages/comments
  • Code maintenance/cleanup

The start time is needed as part of upcoming work on walltime-based worker
draining (so this PR makes it into an instance attribute)...

... and it can be made more accurate than the previous implementation by
measuring it earlier (most especially, prior to invoking the often expensive
probe_addresses call)

This PR rephrases a nearby log line which uses the word "starting" despite
being not in the start() procedure of the manager, to reduce ambiguity when
reading the code vs logs.
@benclifford benclifford merged commit adf2f11 into master Feb 29, 2024
6 checks passed
@benclifford benclifford deleted the benc-ghent-htex-start-time branch February 29, 2024 14:53
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.

2 participants