Ian Maxon has posted comments on this change. Change subject: Implemented LSM disk components alignment ......................................................................
Patch Set 7: (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: needsAlignment Could this name be changed? It is a bit misleading, it implies to me at least that if this were true it is somehow out of alignment. PS7, Line 337: getPartitionID Is there a better way to get this info? Shouldn't it be visible from somewhere above the index itself? 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? PS7, Line 97: offer Why offer? 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? 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: private final String datasetPartitionID; : private final LSMIOOpertionType operationType; : private final List<ILSMIndex> indexes; : private final List<ILSMIndexAccessor> indexAccessors; : private final List<ILSMIOOperation> operations; : private final ILSMIOOperationGroupCallback callback; : : public LSMIOOperationGroup(int datasetID, String partitionID, LSMIOOpertionType operationType, : List<ILSMIndex> indexes, List<ILSMIndexAccessor> indexAccessors, List<ILSMIOOperation> operations, : ILSMIOOperationGroupCallback callback) { : this.datasetPartitionID = datasetID + "-" + partitionID; : this.operationType = operationType; : this.indexes = indexes; : this.operations = operations; : this.indexAccessors = indexAccessors; : this.callback = callback; : : for (ILSMIOOperation operation : this.operations) { : if (this.operationType != operation.getIOOpertionType()) { : throw new IllegalArgumentException("Mismatched operation group type. Operation type" : + operation.getIOOpertionType() + " group operation type: " + this.operationType); : } : operation.setOperationGroup(this); : } : : } Whys all this down here? Can we move it to the top of the class? -- 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: 7 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
