fix(satp-hermes): shutdown of SATP gateway should assure there are no pending transfers before shutting down #3802
Conversation
| @@ -308,6 +309,17 @@ export class SATPManager { | |||
| return this.satpHandlers.get(type); | |||
| } | |||
|
|
|||
| public getSATPSessionState(): boolean { | |||
| private async verifySessionsState(): Promise<void> { | ||
| const fnTag = `${this.className}#verifySessionsState()`; | ||
| this.logger.trace(`Entering ${fnTag}`); | ||
| if (!this.BLODispatcher) { | ||
| throw new Error(`Cannot ${fnTag} because BLODispatcher is erroneous`); | ||
| } | ||
| this.BLODispatcher.setInitiateShutdown(); | ||
| const manager = await this.BLODispatcher.getManager(); | ||
| let status = false; | ||
| while (!status) { | ||
| this.logger.debug(`Inside: ${status}`); | ||
| status = await manager.getSATPSessionState(); | ||
| if (!status) { | ||
| this.logger.info("Sessions are still pending"); | ||
| await new Promise(resolve => setTimeout(resolve, 20000)); | ||
| } | ||
| } | ||
| this.logger.info("All sessions are concluded"); | ||
| } | ||
| } |
There was a problem hiding this comment.
This is a blocker function, change this to use a Cron job:
https://stackoverflow.com/questions/64777543/how-to-make-a-cron-job-for-a-typescript-class-method
| @@ -224,6 +224,12 @@ export class SATPSession { | |||
| return this.serverSessionData?.id || this.clientSessionData?.id || ""; | |||
| } | |||
|
|
|||
| public getSessionState(): State { | |||
| console.log("serverSessionId: ", this.serverSessionData?.state); | |||
There was a problem hiding this comment.
please use the component logger instead of console.log
| this.BLODispatcher.setInitiateShutdown(); | ||
| const manager = await this.BLODispatcher.getManager(); | ||
| let status = false; | ||
| while (!status) { |
There was a problem hiding this comment.
there is a potential infinite loop. this function might block, and we need to decide if this is desirable behaviour. On one hand, the gateway cannot handle anything else; on the other, it should not serve new requests. So there should be a middle-term that allows the gateway to keep serving other requests (namely admin) while not initiating new transfers nor blocking.
Besides, we might want to include a timeout, since something can be broken in the manager (in particular if there is a hanging SATP transfer, communicating with a counterparty gateway).
RafaelAPB
requested review from
petermetz,
takeutak,
izuru0,
jagpreetsinghsasan,
VRamakrishna,
sandeepnRES and
outSH
as code owners
Signed-off-by: Joao Pereira <[email protected]>
joaoecsde
force-pushed
the
shutdownVerify
branch
from
0c911a2 to
54602e8
Compare
Signed-off-by: Joao Pereira <[email protected]>
Introduces a new function,
verifySessionState, which upon shutdown verifies if all sessions are concluded. Also a flag was created to prevent new transactions to occur when shutdown was already called.Additionally, a new type of error is introduced
gateway-error.Resolves #3756.
Pull Request Requirements
upstream/mainbranch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-sflag when usinggit commitcommand. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.