>From Murtadha Hubail <[email protected]>: Attention is currently required from: Ali Alsuliman, Savyasach Reddy. Murtadha Hubail has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187 )
Change subject: [NO ISSUE]: Avoid halts on IO operations failures ...................................................................... Patch Set 23: (15 comments) Commit Message: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/9f54d975_812c36e2 PS23, Line 9: yes this change is internal and not user facing. I would make this "no" File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/nc/HaltCallback.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/ed94154a_3e57dd24 PS23, Line 44: haltOnFailure(operation) you should just replace all of this by operation#shouldHaltOnFailure() and remove haltOnFailure in this class File asterixdb/asterix-column/src/main/java/org/apache/asterix/column/metadata/schema/ObjectSchemaNode.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/187cbc00_44511d44 PS23, Line 167: input.readByte(); Please add a comment as to why this is needed (e.g., what are we skipping). File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/LSMIOOperationCallback.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/afbd2a5d_88d11800 PS23, Line 151: getOperationMaskFilePath I'm wondering if we need to halt for this or just let it be there until the next clean up on start up or a merge re-try assuming we will just replace the mask in case of a re-try. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/59d018ff_801c4b8a PS23, Line 155: ((MergeOperation) operation) mergeOperation File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IPageManager.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/c9643f14_3ad87ddb PS23, Line 51: returnAllPages add java docs File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIOOperation.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/fa0ad38c_df4092ce PS23, Line 183: shouldHalt rename it to shouldHaltOnFailure and add java docs File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIOOperationCallback.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/6b33b2da_572403ce PS23, Line 61: afterFailure Add java docs File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMDiskComponent.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/a3ce2730_18e12976 PS23, Line 180: getIndex().activate(); Is there a chance of failure after this? If yes, then this will set the active flag of the index and any subsequent activate attempts will just fail with index is already active. Do we take care of this some where up the stack? File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/66f0d5f8_ea62d1c7 PS23, Line 130: policy.retry Do we log anything about the re-try attempts (e.g., attempt # or backoff)? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/f0e0ef37_a4487c30 PS23, Line 131: operation are you purposefully not resetting the failure to collect the failure of all attempts? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/6db3371c_d2438497 PS23, Line 134: break; what happens here? File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MergeOperation.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/795d2cad_bebf6de8 PS23, Line 38: shouldHalt make this volatile File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/util/ComponentUtils.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/752a4334_62437896 PS23, Line 177: ignored when can these happen and why are they ignored? File hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187/comment/05c600ac_699df014 PS23, Line 502: c != null In which case c is null? -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19187 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: If253ea03f5baecbab226e527abb4267670a4233e Gerrit-Change-Number: 19187 Gerrit-PatchSet: 23 Gerrit-Owner: Savyasach Reddy <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Ritik Raj <[email protected]> Gerrit-CC: Murtadha Hubail <[email protected]> Gerrit-Attention: Ali Alsuliman <[email protected]> Gerrit-Attention: Savyasach Reddy <[email protected]> Gerrit-Comment-Date: Wed, 19 Feb 2025 09:30:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
