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

Use CommandClient for interchange port reporting instead of Queue #3461

Merged
merged 2 commits into from
May 28, 2024

Conversation

benclifford
Copy link
Collaborator

Before this PR, the interchange used a multiprocessing.Queue to send a single message containing the ports it is listening on back to the submitting process. This ties the interchange into being forked via multiprocessing, even though the rest of the interchange is architected to be forked anyhow, as part of earlier remote-interchange work.

After this PR, the CommandClient used for other submit-side to interchange communication is used to retrieve this information, removing that reliance on multiprocessing and reducing the number of different communication channels used between the interchange and submit side by one.

See issue #3373 for more context on launching the interchange via fork/exec rather than using multiprocessing.

Changed Behaviour

The CommandClient is not threadsafe - see #3376 - and it is possible that this will introduce a new thread-unsafety, as this will be called from the main thread of execution, and most invocations happen (later on) from the status poller thread.

Type of change

  • New feature

Before this PR, the interchange used a multiprocessing.Queue to send a single
message containing the ports it is listening on back to the submitting
process. This ties the interchange into being forked via multiprocessing,
even though the rest of the interchange is architected to be forked anyhow,
as part of earlier remote-interchange work.

After this PR, the CommandClient used for other submit-side to interchange
communication is used to retrieve this information, removing that reliance
on multiprocessing and reducing the number of different communication channels
used between the interchange and submit side by one.

See issue #3373 for more context on launching the interchange via fork/exec
rather than using multiprocessing.
@benclifford benclifford requested review from yadudoc and rjmello May 27, 2024 11:11
Copy link
Collaborator

@khk-globus khk-globus left a comment

Choose a reason for hiding this comment

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

Nice cleanup; good use of the other infrastructure already in place.

@benclifford benclifford merged commit 0ad1587 into master May 28, 2024
6 checks passed
@benclifford benclifford deleted the benc-interchange-port-report branch May 28, 2024 19:58
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