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

Separate heartbeat_periods and reporting_period #1356

Merged
merged 6 commits into from
Nov 9, 2023

Conversation

yadudoc
Copy link
Collaborator

@yadudoc yadudoc commented Nov 8, 2023

Description

The GlobusComputeEngine has a notion of hearbeat_period and heartbeat_threshold used internally to track worker liveness. This is separate from the status reporting logic implemented in the GlobusComputeEngineBase that sends EPStatusReports.

This PR sets the reporting period to a constant. Users should not be able to tweak this, and we should not be supporting cases where there's any functionality built on faster updates.

Fixes # (issue)

Type of change

Choose which options apply, and delete the ones which do not apply.

  • Code maintenance/cleanup

@yadudoc yadudoc requested a review from rjmello November 8, 2023 21:23
Copy link
Member

@rjmello rjmello left a comment

Choose a reason for hiding this comment

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

We'll also need to update the engine_runner fixture and the status report tests.

@yadudoc yadudoc force-pushed the heartbeats_and_reporting_sc_28343 branch from 7217414 to eb6f4fb Compare November 8, 2023 22:56
@yadudoc yadudoc force-pushed the heartbeats_and_reporting_sc_28343 branch 2 times, most recently from d7ad2c6 to 64ae4ed Compare November 9, 2023 00:12
* Test fixes
* Removing heartbeat params from engine init
* Minor perf improvements
@yadudoc yadudoc force-pushed the heartbeats_and_reporting_sc_28343 branch from 64ae4ed to d2ca73f Compare November 9, 2023 00:34
Copy link
Member

@rjmello rjmello left a comment

Choose a reason for hiding this comment

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

Nice!

@rjmello
Copy link
Member

rjmello commented Nov 9, 2023

[sc-28343]

Copy link

@yadudoc yadudoc merged commit 89da1f4 into main Nov 9, 2023
30 checks passed
@yadudoc yadudoc deleted the heartbeats_and_reporting_sc_28343 branch November 9, 2023 16:29
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