Luo Chen has posted comments on this change. Change subject: Implemented LSM disk components alignment ......................................................................
Patch Set 8: (6 comments) > (6 comments) https://asterix-gerrit.ics.uci.edu/#/c/1725/7/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndex.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndex.java: PS7, Line 332: > Could this name be changed? It is a bit misleading, it implies to me at lea What about 'isCorrelated'? PS7, Line 337: > Is there a better way to get this info? Shouldn't it be visible from somewh I carried 'partition' info when constructing LSMIndex (which causes a lot of code changes). Would these changes be OK? https://asterix-gerrit.ics.uci.edu/#/c/1725/7/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AsynchronousScheduler.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AsynchronousScheduler.java: PS7, Line 59: waitingOperationGroups > Isnt there concurrent access to this variable? There should be no concurrent access to this variable (all accesses of this map is synchronized under the executor object). PS7, Line 97: offer > Why offer? Just to be consistent with the previous 'runningFlushOperations' (line 80). Since it is LinkedList, the offer operation would always succeed. https://asterix-gerrit.ics.uci.edu/#/c/1725/7/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: PS7, Line 458: ); > Why 0? If it is a flush operation, then ctx.getComponentHolder() would only contain the exact one memory component to be flushed. (This function is summarized from 'getAndEnterComponents' for checking whether a component is ready to flush). https://asterix-gerrit.ics.uci.edu/#/c/1725/7/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMIOOperationGroup.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMIOOperationGroup.java: PS7, Line 92: } : } : } : if (!ready) { : return null; : } : List<ILSMIndex> indexes = new ArrayList<>(indexSet.size()); : List<ILSMIOOperation> operations = new ArrayList<>(indexSet.size()); : : for (ILSMIndex lsmIndex : indexSet) { : if (lsmIndex.getPartition() == partitionID) { : //get resource : //schedule flush after update : ILSMIOOperation operation = : indexAccessors.get(indexes.size()).createFlushOperation(lsmIndex.getIOOperationCallback()); : if (operation == null) { : throw new IllegalStateException("Inconsistent memory component state for dataset " + datasetID); : } : operations.add(operation); : indexes.add(lsmIndex); : } : } : : return new LSMIOOperationGroup(datasetID, partitionID, LSMIOOpertionType.FLUSH, indexes, indexAccessors, : operations, callback); : } > Whys all this down here? Can we move it to the top of the class? Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1725 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I64bf34e255def72adc73b9f87cfa628a172ea694 Gerrit-PatchSet: 8 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Luo Chen <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Luo Chen <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
