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

Wait for all session workers to finish #275

Merged

Conversation

alcarraz
Copy link
Contributor

before shutting down the server.

When sessions property was greater than one, the first to end was shutting down the Q2 server whether the others have finished or not.

before shutting down the server.
@ar
Copy link
Member

ar commented Dec 31, 2022

Great PR that addresses a real problem, and I love latches as it reminds me TTL logic days, but couldn't you just awaitTermination on the executor?

@alcarraz
Copy link
Contributor Author

alcarraz commented Jan 2, 2023

Great PR that addresses a real problem, and I love latches as it reminds me TTL logic days, but couldn't you just awaitTermination on the executor?

As a matter of fact, that was my first approach. It was failing for an unrelated issue (some silly error of mine relating to setting the timeout variable in the wrong place) and while debugging that issue, I added the latch. I was debating with myself about leaving the latch or get back to using the executor's awaitTermination. Somehow I believed the latch could provide a clearer intention, but I'm with you in that less is more.

Use `ExecutorService.awaitTermination` instead.
@alcarraz alcarraz force-pushed the feature/client-simulator-await-sessions-end branch from c8bb67e to ad341a9 Compare January 2, 2023 17:46
Forgot to remove import in last commit.
@alcarraz
Copy link
Contributor Author

alcarraz commented Jan 5, 2023

FYI, suggested change has already been pushed to the PR.

Great PR that addresses a real problem, and I love latches as it reminds me TTL logic days, but couldn't you just awaitTermination on the executor?

As a matter of fact, that was my first approach. It was failing for an unrelated issue (some silly error of mine relating to setting the timeout variable in the wrong place) and while debugging that issue, I added the latch. I was debating with myself about leaving the latch or get back to using the executor's awaitTermination. Somehow I believed the latch could provide a clearer intention, but I'm with you in that less is more.

@ar ar merged commit b167f90 into jpos:master Jan 5, 2023
@alcarraz alcarraz deleted the feature/client-simulator-await-sessions-end branch February 1, 2023 00:28
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