Ian Maxon has posted comments on this change. Change subject: [ASTERIXDB-1952][TX][IDX] Filter logs pt.2 ......................................................................
Patch Set 26: (24 comments) https://asterix-gerrit.ics.uci.edu/#/c/1857/26/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/nc/RecoveryManager.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/nc/RecoveryManager.java: PS26, Line 280: @SuppressWarnings({"squid:MethodCyclomaticComplexity","squid:S134"}) > remove this. It is better that we get the warning so that we know this is a Done https://asterix-gerrit.ics.uci.edu/#/c/1857/26/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/LogRecord.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/LogRecord.java: Line 127: case LogType.ENTITY_COMMIT: > writeEntityResource Done PS26, Line 168: writeEntityInfo > writeEntityValue Done PS26, Line 169: buffer.putInt(resourcePartition); : buffer.putInt(datasetId); > remove Done PS26, Line 179: writeEntityInfoNoPK > writeEntityResource Done Line 275: case LogType.ENTITY_COMMIT: > readEntityResource See corresponding comment for write... PS26, Line 293: readEntityNoPKInfo > readEntityResource Done PS26, Line 294: RecordReadStatus updStatus = > return Done PS26, Line 330: readEntityInfo > readEntityValue Done PS26, Line 335: resourcePartition = buffer.getInt(); : datasetId = buffer.getInt(); > remove Done PS26, Line 350: readEntityNoPKInfo > readEntityResource Done PS26, Line 351: //attempt to read in the resourcePartition, dsid, PK hash and PK length > fix comment Done PS26, Line 352: ENTITYCOMMIT_UPDATE_HEADER_LEN > this isn't correct. You need a new one for entity resource length, then adj Done https://asterix-gerrit.ics.uci.edu/#/c/1857/26/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java: PS26, Line 170: true > revert Done https://asterix-gerrit.ics.uci.edu/#/c/1857/26/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/AbstractIndexModificationOperationCallback.java File asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/AbstractIndexModificationOperationCallback.java: PS26, Line 127: public void after(ITupleReference newValue) > have an explicit call to log the filter's new min and max Why would that be superior to simply logging the value (or not) as it comes through the modification pipeline, as it is now? PS26, Line 135: catch (ACIDException e) { : throw new HyracksDataException(e); : } > ACIDException is a runtime exception now, you don't need to wrap it. Done https://asterix-gerrit.ics.uci.edu/#/c/1857/26/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/ophelpers/IndexOperation.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/ophelpers/IndexOperation.java: PS26, Line 28: FILTER_MOD > you shouldn't need this It is necessary because this value is used in the log during redo. https://asterix-gerrit.ics.uci.edu/#/c/1857/26/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMHarness.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMHarness.java: PS26, Line 229: /** : * Update the filter with the value in the passed tuple, also give the tuple to the modification callback : * : * @param ctx : * @throws HyracksDataException : */ : void updateFilter(ILSMIndexOperationContext ctx, ITupleReference tuple, boolean callback) : throws HyracksDataException; > You shouldn't need this Done https://asterix-gerrit.ics.uci.edu/#/c/1857/26/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java: PS26, Line 721: @Override : public void updateFilter(ILSMIndexOperationContext ctx, ITupleReference tuple, boolean callback) : throws HyracksDataException { : if (!callback) { : updateFilter(ctx, tuple); : return; : } : : if (!lsmIndex.isMemoryComponentsAllocated()) { : lsmIndex.allocateMemoryComponents(); : } : ctx.getModificationCallback().after(tuple); : lsmIndex.updateFilter(ctx, tuple); : } > remove Done https://asterix-gerrit.ics.uci.edu/#/c/1857/26/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java: PS26, Line 214: public void updateFilter(ITupleReference tuple, boolean callback) throws HyracksDataException { : ctx.setOperation(IndexOperation.UPSERT); : lsmHarness.updateFilter(ctx, tuple, callback); : } > remove Done https://asterix-gerrit.ics.uci.edu/#/c/1857/26/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/IModificationOperationCallback.java File hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/IModificationOperationCallback.java: PS26, Line 62: void after(ITupleReference after) throws HyracksDataException; > If you do this here, then you have to stick to the contract of this interfa Roughly done. I had to name it something else and keep it in am.common, because otherwise it turns into a interface-breaking quagmire. https://asterix-gerrit.ics.uci.edu/#/c/1857/26/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/btree/AbstractModificationOperationCallbackTest.java File hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/btree/AbstractModificationOperationCallbackTest.java: PS26, Line 104: @Override : public void after(ITupleReference tuple) throws HyracksDataException { : Assert.assertEquals(0, cmp.compare(AbstractModificationOperationCallbackTest.this.tuple, tuple)); : } > you shouldn't need this Done https://asterix-gerrit.ics.uci.edu/#/c/1857/26/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/common/TestOperationCallback.java File hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/common/TestOperationCallback.java: PS26, Line 69: @Override : public void after(ITupleReference tuple) throws HyracksDataException { : // Do nothing. : } > you shouldn't need this Done https://asterix-gerrit.ics.uci.edu/#/c/1857/26/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/LSMBTreeUpdateInPlaceTest.java File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/LSMBTreeUpdateInPlaceTest.java: PS26, Line 180: @Override : public void after(ITupleReference tuple) { : : } > you shouldn't need this Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1857 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie9e7795d9c8c212e8610dcb9bb5d26ec9fbbee8a Gerrit-PatchSet: 26 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ian Maxon <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Luo Chen <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
