Murtadha Hubail has posted comments on this change. Change subject: [NO ISSUE][TX] Make TxnLogFile Close Idempotent ......................................................................
Patch Set 1: (3 comments) https://asterix-gerrit.ics.uci.edu/#/c/2165/1/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/logging/LogManager.java File asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/logging/LogManager.java: PS1, Line 582: : : > Should we warn here? Or do we need to consider this a "regular case"? It is a rare regular case. Logging it wouldn't do any harm and might help us in debugging one day. I will add the log. https://asterix-gerrit.ics.uci.edu/#/c/2165/1/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/logging/LogReader.java File asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/logging/LogReader.java: PS1, Line 150: ACIDException > What happens if we stop here instead of ignoring the interrupt and continui This would be a txn abort thread waiting for the flushLSN. If such thread is interrupt, it means that we are shutting down the NC, therefore, when the NC starts up, recovery will redo the committed records and there wont be anything to undo. I think this is better than leaving the thread waiting during shutdown. PS1, Line 288: logFile.close() > Should we set logFile to null here or do we still need it? This is assuming that the caller would adhere to the contract of ILogReader (i.e. close will be called when the logReader will no longer be used. However, calling close twice on the same log file has no effect since there is protection against that in TxnLogFile#close. It is similar to calling close multiple times on a file channel. Setting it to null wouldn't harm too. Done. -- To view, visit https://asterix-gerrit.ics.uci.edu/2165 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I255e4b9af0bc78298c0a25daf0b5629d413eba6a Gerrit-PatchSet: 1 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
