Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Reduce the shutdown_cleanup_period. #473

Merged
merged 1 commit into from
Nov 16, 2020

Conversation

clalancette
Copy link
Contributor

This should greatly reduce how long it takes to shutdown a
Connext Node. In local testing the time went from 3 seconds
to well under a second.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

This should fix #325; an open question is whether this has any other side-effects. I'm going to run a full CI with this change in place, and Connext as the only RMW vendor, to see what happens.

@clalancette clalancette requested a review from brawner October 29, 2020 16:09
@clalancette clalancette marked this pull request as draft October 29, 2020 16:09
@clalancette
Copy link
Contributor Author

clalancette commented Oct 29, 2020

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status <-- expected to fail, no connext on aarch64
  • macOS Build Status
  • Windows Build Status

Copy link

@brawner brawner left a comment

Choose a reason for hiding this comment

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

Excellent, thanks Chris for doing this

This should greatly reduce how long it takes to shutdown a
Connext Node.  In local testing the time went from 3 seconds
to well under a second.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette force-pushed the clalancette/reduce-cleanup-period branch from 951dad7 to 568765a Compare November 16, 2020 16:09
@clalancette
Copy link
Contributor Author

Another CI, hopefully a little more successful:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@clalancette clalancette marked this pull request as ready for review November 16, 2020 19:07
@clalancette
Copy link
Contributor Author

The Windows failures are well-known by now, so almost certainly not the result of this PR. I was sort of hoping the quicker shutdown type would help this, but it was not to be. In any case, I'm going to merge this one as-is, and we'll deal with the failures elsewhere.

@clalancette clalancette merged commit 7d32e76 into master Nov 16, 2020
@delete-merged-branch delete-merged-branch bot deleted the clalancette/reduce-cleanup-period branch November 16, 2020 22:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connext is very slow to shutdown
2 participants