Yingyi Bu has posted comments on this change.
Change subject: Add Create Secondary BTree for Correlated Datasets
......................................................................
Patch Set 10:
(16 comments)
It looks that new bulkload code hasn't actually been run?
Check the following method in IndexUtil:
public static JobSpecification buildSecondaryIndexLoadingJobSpec(Dataset
dataset, Index index,
MetadataProvider metadataProvider) throws AlgebricksException {
SecondaryIndexOperationsHelper secondaryIndexHelper =
SecondaryIndexOperationsHelper.createIndexOperationsHelper(dataset, index,
metadataProvider,
physicalOptimizationConfig);
return secondaryIndexHelper.buildLoadingJobSpec();
}
That shows how harmful the static method is? Maybe it's time to refactor the
SecondaryIndexHelper using the factory pattern?
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java
File
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java:
PS10, Line 959: IndexUtil.buildSecondaryIndexLoadingJobSpec(ds, index,
metadataProvider);
Inside this method, IndexUtil still uses the old
SecondaryIndexOperationsHelper, even for correlated datasets.
Thus, it looks to me that the new bulkload code path hasn't been executed.
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.1.ddl.aql
File
asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.1.ddl.aql:
PS10, Line 21: the correlated prefix merge policy guarantees that disk
components of
: * the primary index and the secondary indexes of a dataset are
always aligned.
It only tests the query correctness of correlated indexes? Disk layout is not
tested here.
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.2.update.aql
File
asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.2.update.aql:
PS10, Line 21: merge policy guarantees that disk components of
: * the primary index and the secondary indexes of a dataset are
always aligned.
This particular aspect is not tested here. What is tested here is the
correctness of correlated indexes.
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.4.query.aql
File
asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.4.query.aql:
PS10, Line 21: merge policy guarantees that disk components of
: * the primary index and the secondary indexes of a dataset are
always aligned
The disk component alignment is not checked by this test. Instead, this test
checks that indexes with correlated merge policy can deliver correct results.
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-app/src/test/resources/runtimets/queries/filters/insert-with-correlated-secondary-btree/insert-with-correlated-secondary-btree.1.ddl.aql
File
asterixdb/asterix-app/src/test/resources/runtimets/queries/filters/insert-with-correlated-secondary-btree/insert-with-correlated-secondary-btree.1.ddl.aql:
PS10, Line 44:
WS
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-app/src/test/resources/runtimets/queries/temp-dataset/insert-and-scan-dataset-with-correlated-index/insert-and-scan-dataset-with-correlated-index.1.ddl.aql
File
asterixdb/asterix-app/src/test/resources/runtimets/queries/temp-dataset/insert-and-scan-dataset-with-correlated-index/insert-and-scan-dataset-with-correlated-index.1.ddl.aql:
PS10, Line 20:
WS
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-app/src/test/resources/runtimets/queries/temp-dataset/insert-and-scan-dataset-with-correlated-index/insert-and-scan-dataset-with-correlated-index.2.update.aql
File
asterixdb/asterix-app/src/test/resources/runtimets/queries/temp-dataset/insert-and-scan-dataset-with-correlated-index/insert-and-scan-dataset-with-correlated-index.2.update.aql:
PS10, Line 20:
WS
PS10, Line 22: deadlatch
deadlock or dead latch?
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-app/src/test/resources/runtimets/queries/temp-dataset/insert-and-scan-dataset-with-correlated-index/insert-and-scan-dataset-with-correlated-index.3.ddl.aql
File
asterixdb/asterix-app/src/test/resources/runtimets/queries/temp-dataset/insert-and-scan-dataset-with-correlated-index/insert-and-scan-dataset-with-correlated-index.3.ddl.aql:
PS10, Line 20:
Set IDE to trim WS?
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-app/src/test/resources/runtimets/queries/temp-dataset/insert-and-scan-dataset-with-correlated-index/insert-and-scan-dataset-with-correlated-index.4.query.aql
File
asterixdb/asterix-app/src/test/resources/runtimets/queries/temp-dataset/insert-and-scan-dataset-with-correlated-index/insert-and-scan-dataset-with-correlated-index.4.query.aql:
PS10, Line 21: e we insert a materializing to prevent the possib
materialization is no longer need.
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/utils/SecondaryCorrelatedTreeIndexOperationsHelper.java
File
asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/utils/SecondaryCorrelatedTreeIndexOperationsHelper.java:
PS10, Line 89: -
why negative here?
PS10, Line 290: createIndexOperationsHelper
It looks no one calls this method?
That explains why the new bulkload code path hasn't been executed?
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/AbstractLSMSecondaryCreationNodePushable.java
File
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/AbstractLSMSecondaryCreationNodePushable.java:
PS10, Line 31: AbstractLSMSecondaryCreationNodePushable
AbstractLSMSecondaryCreationNodePushable ->
AbstractLSMSecondaryIndexCreationNodePushable
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryCreationBulkLoadNodePushable.java
File
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryCreationBulkLoadNodePushable.java:
PS10, Line 46: Creation
Creation -> Index
https://asterix-gerrit.ics.uci.edu/#/c/1813/10/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/LSMBTreeDiskComponentScanOperatorNodePushable.java
File
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/LSMBTreeDiskComponentScanOperatorNodePushable.java:
PS10, Line 54: return null;
Throw UnSupportedException, if no one should call into this method for this
class?
PS10, Line 60: need
Throw UnSupportedException, if no one should call into this method for this
class?
--
To view, visit https://asterix-gerrit.ics.uci.edu/1813
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a3435e6720f07bd6a5092d4d9ce42e8d4b7894c
Gerrit-PatchSet: 10
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
