Luo Chen has posted comments on this change.

Change subject: Add Create Secondary BTree for Correlated Datasets
......................................................................


Patch Set 10:

(5 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1813/1/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/utils/SecondaryCorrelatedBTreeOperationsHelper.java
File 
asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/utils/SecondaryCorrelatedBTreeOperationsHelper.java:

Line 71:         assert dataset.getDatasetType() == DatasetType.INTERNAL;
> MAJOR SonarQube violation:
Done


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 305:      //TODO Inverted Index
              :                 //TODO This will be fixed soon
> Hm, what should be done for text and rtree index? At a high level...
The basic workflow is similar, i.e., scan the primary index, extract secondary 
keys + primary key + filter, post-processor tuples (for generate anti-matter 
tuples for upsert), external sort and bulk load.

Two difference:
1. for RTree (wit buddy btree), the anti-matter tuple only contains the primary 
key without secondary key (this is already implemented in the 
LSMSecondaryCreationTupleProcessor operator).
2. for inverted index, we need a tokenization step before external sort to 
tokenize the text (secondary key).


https://asterix-gerrit.ics.uci.edu/#/c/1813/1/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryCreationTupleProcessorNodePushable.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryCreationTupleProcessorNodePushable.java:

Line 100:     public 
LSMSecondaryCreationTupleProcessorNodePushable(IHyracksTaskContext ctx, 
RecordDescriptor inputRecDesc,
> MAJOR SonarQube violation:
Done


https://asterix-gerrit.ics.uci.edu/#/c/1813/1/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:

Line 58:     protected void resetSearchPredicate(int tupleIndex) {
> MAJOR SonarQube violation:
Done


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/impls/LSMBTreeDiskComponentScanCursor.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeDiskComponentScanCursor.java:

PS10, Line 153:    // This could happen sometimes...
              :             // Consider insert tuple A into the memory 
component, and then delete it immediately.
              :             // We would have -A in the memory component, but 
there is no tuple A in the previous disk components.
              :             // But in this case, we can simply ignore it for 
the scan purpose
              :             return false;
> Wouldnt inserting A and then deleting it while it's still in memory not res
When delete a tuple, we should always place a anti-matter for that tuple in the 
memory component (unless that tuple is not in the dataset).

The reason is that what if A is already in the disk component? Not setting the 
anti-matter tuple in the memory component makes A in the disk component visible 
again...


-- 
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

Reply via email to