Luo Chen has posted comments on this change. Change subject: Implemented LSM disk components alignment ......................................................................
Patch Set 18: (4 comments) > (24 comments) > > half way through this but I think this is enough for this patch... https://asterix-gerrit.ics.uci.edu/#/c/1725/17/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/CorrelatedPrefixMergePolicy.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/CorrelatedPrefixMergePolicy.java: PS17, Line 94: lsmIndex.getPartition() == index.getPartition()) { : if (numComponents != lsmIndex.getImmutableComponents().size()) { : RuntimeDataException.create(ErrorCode.INDEX_NOT_ALIGNED, lsmIndex.toString(), index.toString()); : } > This will end up being thrown a lot. For example, if there is an secondary Got it! The current implementation based on a critical assumption that whenever the merge policy is called, the primary index should always have the same number of disk components w.r.t. secondaries. However, it seems this assumption is very difficult to achieve, and makes the whole solution very brittle (once the assumption is broken, the whole thing won't work...) I might need to reconsider the whole solution based on this fact... https://asterix-gerrit.ics.uci.edu/#/c/1725/17/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetInfo.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetInfo.java: PS17, Line 79: public synchronized ILSMIndex getPrimaryIndex() { : Set<ILSMIndex> indexSet = getDatasetIndexes(); : for (ILSMIndex index : indexSet) { : if (index.isPrimaryIndex()) { : return index; : } : } : return null; : } > What is the intended behavior of this method? can we add a comment? Simply get the primary index for this dataset (as a helper method). https://asterix-gerrit.ics.uci.edu/#/c/1725/17/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java: PS17, Line 411: Set<ILSMIndex> indexSet = dsInfo.getDatasetIndexes(); : ILSMIndex primaryIndex = dsInfo.getPrimaryIndex(); : if (primaryIndex.isCorrelated()) { : List<ILSMIOOperationGroup> operationGroups = : LSMIOOperationGroup.create(dsInfo.getDatasetID(), indexSet, new LSMIOOperationGroupCallback()); : for (ILSMIOOperationGroup operationGroup : operationGroups) { : primaryIndex.getIOScheduler().scheduleOperationGroup(operationGroup); : } : } else { : for (ILSMIndex lsmIndex : indexSet) { : ILSMIndexAccessor accessor = : lsmIndex.createAccessor(NoOpOperationCallback.INSTANCE, NoOpOperationCallback.INSTANCE); : accessor.scheduleFlush(lsmIndex.getIOOperationCallback()); : } > This kind of code with if else is bad. essentially, the code should be the Ideally, it should be. But the current solution requires all indexes be flushed together in a group, which thus requires such if-else statements. https://asterix-gerrit.ics.uci.edu/#/c/1725/17/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/ExternalBTreeLocalResourceMetadata.java File asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/ExternalBTreeLocalResourceMetadata.java: PS17, Line 64: partition() > Storage in Hyracks has been designed to be be agnostic to storage partition Got it. The current solution requires knowing the partition id of each index. Maybe I can use the recent change (separate index creation with access) to get rid of this. -- 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: 18 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
