abdullah alamoudi has posted comments on this change. Change subject: Separate index build from index access ......................................................................
Patch Set 7: (19 comments) https://asterix-gerrit.ics.uci.edu/#/c/1728/4/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/CorrelatedPrefixMergePolicyFactory.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/CorrelatedPrefixMergePolicyFactory.java: PS4, Line 53: > Can we move this set call into the constructor and make datasetId a final? Done https://asterix-gerrit.ics.uci.edu/#/c/1728/4/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/DatasetLocalResource.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/dataflow/DatasetLocalResource.java: PS4, Line 42: DatasetLocalResource > Why IResource/DatasetLocalREsource are serializable? This is needed to go to disk since the partition and datasetId are used in recovery... https://asterix-gerrit.ics.uci.edu/#/c/1728/4/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/BTreeResourceFactoryProvider.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/BTreeResourceFactoryProvider.java: PS4, Line 112: aits; > static methods should go to XxxUtil? made them private because they're only used here. kept them static because they are stateless PS4, Line 149: p > static methods should go to XxxUtil? made them private because they're only used here. kept them static because they are stateless PS4, Line 183: } else > static methods should go to XxxUtil? made them private because they're only used here. kept them static because they are stateless https://asterix-gerrit.ics.uci.edu/#/c/1728/4/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java: PS4, Line 632: > Add this to IDataset? Done PS4, Line 639: Provider storageComp > Add this to IDataset interface? requires a lot of moving things around because of classes in signature PS4, Line 668: Factories = new IBinaryCompar > Add this to IDataset? requires a lot of moving things around because of classes in signature PS4, Line 693: > Add this to IDataset? Done https://asterix-gerrit.ics.uci.edu/#/c/1728/4/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java File asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java: PS4, Line 153: cksDataException.crea > Error code? Done https://asterix-gerrit.ics.uci.edu/#/c/1728/4/hyracks-fullstack/algebricks/algebricks-data/src/main/java/org/apache/hyracks/algebricks/data/IBinaryComparatorFactoryProvider.java File hyracks-fullstack/algebricks/algebricks-data/src/main/java/org/apache/hyracks/algebricks/data/IBinaryComparatorFactoryProvider.java: PS4, Line 51: > Unify the two interface method into one? Issue created! https://issues.apache.org/jira/browse/ASTERIXDB-1907 https://asterix-gerrit.ics.uci.edu/#/c/1728/4/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexFrame.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexFrame.java: PS4, Line 39: ; > why 5->6? because the serialized java files have changed and so the system will not be able to run on previous version's data. to announce this, we increment the version https://asterix-gerrit.ics.uci.edu/#/c/1728/4/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexHelper.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexHelper.java: PS4, Line 33: > IndexHelper -> IndexDataflowHelper Done https://asterix-gerrit.ics.uci.edu/#/c/1728/4/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeWithBuddy.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeWithBuddy.java: PS4, Line 805: setCurrentVersion > Can this setXXX be done in the constructor of the Index? In this way, we d This can't be done like that unfortunately without considerable amount of work. it used to be like that before but now that the way we create the indexes is always from the serialized resource, we can't do this. We can do another way where we allow the creator of the resource to pass more info but it is not a small change so I thought we keep it till later. https://asterix-gerrit.ics.uci.edu/#/c/1728/4/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ITwoPCIndex.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ITwoPCIndex.java: PS4, Line 80: setCurrentVersion > Can we not expose setCurrentVersion and instead do that in the constructor commented on this in the implementation. unfortunately it is not as straight forward as I'd like it to be. https://asterix-gerrit.ics.uci.edu/#/c/1728/4/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/IVirtualBufferCacheProvider.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/IVirtualBufferCacheProvider.java: PS4, Line 32: : : : : > I searched in the codebase, but couldn't find callers of the first two meth Cool. removed!! https://asterix-gerrit.ics.uci.edu/#/c/1728/4/hyracks-fullstack/hyracks/hyracks-storage-common/pom.xml File hyracks-fullstack/hyracks/hyracks-storage-common/pom.xml: PS4, Line 51: dataflow > Why storage should depend on dataflow? In principle, storage is library an because tupleFilter is used in the filter factory which is in some of the index operators. and it uses IFrameTupleReference which is in dataflow https://asterix-gerrit.ics.uci.edu/#/c/1728/4/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-common-test/src/test/java/org/apache/hyracks/storage/common/BufferCacheTest.java File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-common-test/src/test/java/org/apache/hyracks/storage/common/BufferCacheTest.java: PS4, Line 107: > WS Done PS4, Line 115: > WS Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1728 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie4ea3aaa63dff8d246fa43ca7c7359729bc8cf47 Gerrit-PatchSet: 7 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
