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

Mixnode gracefully shutdown #330

Closed
wants to merge 18 commits into from
Closed

Mixnode gracefully shutdown #330

wants to merge 18 commits into from

Conversation

al8n
Copy link
Contributor

@al8n al8n commented Aug 28, 2023

Add a MixnodeHandle struct to deal with gracefully shutdown logic in mix node. With this change, we can also access the actual address the mix node is listening.

@al8n al8n added enhancement New feature or request mixnet labels Aug 28, 2023
@al8n al8n added this to the Mixnet milestone Aug 28, 2023
@al8n al8n self-assigned this Aug 28, 2023
@al8n al8n changed the base branch from master to integration-auto-port August 28, 2023 03:34
@al8n al8n changed the base branch from integration-auto-port to mixnet August 28, 2023 11:04
@youngjoon-lee
Copy link
Contributor

Now that we use Overwatch for mixnode, can we close this PR? It seems nomos-node also doesn't have signal handling, if I understand correctly (I'm not 100% sure since I don't have much experience in Overwatch). What do you think about postponing this issue until we have a consistent way with Overwatch, if we don't have any critical issue?

@al8n
Copy link
Contributor Author

al8n commented Sep 5, 2023

Now that we use Overwatch for mixnode, can we close this PR? It seems nomos-node also doesn't have signal handling, if I understand correctly (I'm not 100% sure since I don't have much experience in Overwatch). What do you think about postponing this issue until we have a consistent way with Overwatch, if we don't have any critical issue?

Yeah, nomos-node does not have signal handling. This PR originally targets adding a Handle to get the correct port when the user gives a 0, so that we can use the criterion for the mixnet benchmark. If we do not need to handle that case anymore, I think we can close this one. Maybe it is better to add a graceful shutdown mechanism in Overwatch.

@danielSanchezQ
Copy link
Collaborator

Now that we use Overwatch for mixnode, can we close this PR? It seems nomos-node also doesn't have signal handling, if I understand correctly (I'm not 100% sure since I don't have much experience in Overwatch). What do you think about postponing this issue until we have a consistent way with Overwatch, if we don't have any critical issue?

Yeah, nomos-node does not have signal handling. This PR originally targets adding a Handle to get the correct port when the user gives a 0, so that we can use the criterion for the mixnet benchmark. If we do not need to handle that case anymore, I think we can close this one. Maybe it is better to add a graceful shutdown mechanism in Overwatch.

Yes, it was one of the issues in Overwatch iirc. If not it was one of the main things that was left to be implemented. I would encourage to implement there so it would benefit every binary.

@youngjoon-lee
Copy link
Contributor

Yup. We can close this PR.

@al8n al8n closed this Sep 6, 2023
@jakubgs jakubgs deleted the mixnet-gracefully-shutdown branch February 15, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request mixnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants