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

.close() interchange process to release more fds #3399

Merged
merged 2 commits into from Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions parsl/executors/high_throughput/executor.py
Expand Up @@ -818,4 +818,6 @@ def shutdown(self, timeout: float = 10.0):
logger.info("Unable to terminate Interchange process; sending SIGKILL")
self.interchange_proc.kill()

self.interchange_proc.close()

logger.info("Finished HighThroughputExecutor shutdown attempt")
4 changes: 0 additions & 4 deletions parsl/tests/test_htex/test_missing_worker.py
Expand Up @@ -37,7 +37,3 @@ def test_that_it_fails():
raise Exception("The app somehow ran without a valid worker")

assert parsl.dfk().config.executors[0]._executor_bad_state.is_set()

# htex needs shutting down explicitly because dfk.cleanup() will not
# do that, as it is in bad state
parsl.dfk().config.executors[0].shutdown()
Comment on lines -40 to -43
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good change ... ought we go one further and include a fixture that fails if the executor is not shutdown at the end of each test (or perhaps module)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is working towards two fixtures that will fail on threads existing and fds being open...
checking that dfk.cleanup() has been called would be pretty straightforward: that flag already exists in order to try to make dfk.cleanup() idempotent. but not all tests have a dfk... or exactly one dfk...