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

fix(satp): shutdown of SATP gateway should assure there are no pending transfers before shutting down #3756

Open
RafaelAPB opened this issue Feb 7, 2025 · 4 comments
Assignees
Labels
bug Something isn't working IETF-SATP-Hermes Related to the Secure Asset Transfer Protocol as defined by the Internet Engineering Task Force.

Comments

@RafaelAPB
Copy link
Contributor

Describe the bug

The shutdown code at https://github.com/hyperledger-cacti/cacti/blob/satp-dev/packages/cactus-plugin-satp-hermes/src/main/typescript/plugin-satp-hermes-gateway.ts#L585 does not clean up existing sessions. It should wait that all transfers are resolved prior to shutdown.

To Reproduce
NA

Expected behavior

A call to gateway.shutdown() should wait for all pending sessions to counterparty gateways to be resolved.

NA

@RafaelAPB RafaelAPB added bug Something isn't working IETF-SATP-Hermes Related to the Secure Asset Transfer Protocol as defined by the Internet Engineering Task Force. labels Feb 7, 2025
@AndreAugusto11 AndreAugusto11 changed the title Shutdown/cleanup of SATP gateway should assure there are no pending transfers before shutting down fix(satp): shutdown of SATP gateway should assure there are no pending transfers before shutting down Feb 7, 2025
@AndreAugusto11 AndreAugusto11 moved this to Backlog in SATP-Hermes Feb 7, 2025
@joaoecsde joaoecsde self-assigned this Feb 8, 2025
@LordKubaya
Copy link
Contributor

@RafaelAPB should we wait forever for the sessions to finish? Should we have a function that forces the gateway to shutdown and triggering rollbacks of the unfinished sessions?

@RafaelAPB
Copy link
Contributor Author

@LordKubaya I'd say we should wait indefinitely because if the crash recovery timeout is triggered then the bad session is handled, there will be no more pending sessions and the gateway can shutdown gracefully. In other words, the gateway should have to wait at most the delta for crash recovery + any extra time the rollback steps take. If this is not respected we can emit warnings/errors but IMO we should wait.

Otherwise we could be unintentionally be causing our gateway to crash from the perspective of other gateways, consequently triggering the (expensive) rollback protocol

@LordKubaya
Copy link
Contributor

LordKubaya commented Feb 14, 2025

@LordKubaya I'd say we should wait indefinitely because if the crash recovery timeout is triggered then the bad session is handled, there will be no more pending sessions and the gateway can shutdown gracefully. In other words, the gateway should have to wait at most the delta for crash recovery + any extra time the rollback steps take. If this is not respected we can emit warnings/errors but IMO we should wait.

Otherwise we could be unintentionally be causing our gateway to crash from the perspective of other gateways, consequently triggering the (expensive) rollback protocol

@RafaelAPB

Ok, that makes sense.
Imagine that we need to shutdown the gateway fast (maintenance, etc), we can have a mecanism to transfer the transfer to other gateway that have support for the transfer we are doing?

@RafaelAPB
Copy link
Contributor Author

I'd say no, if the gateway does not answer it is consider crashed.

@RafaelAPB RafaelAPB moved this from Backlog to In progress in SATP-Hermes Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working IETF-SATP-Hermes Related to the Secure Asset Transfer Protocol as defined by the Internet Engineering Task Force.
Projects
Status: In progress
Development

No branches or pull requests

3 participants