Change in asterixdb[master]: ASTERIXDB-1088: fixed a secondary index compact issue

[Young-Seok Kim (Code Review)]

Young-Seok Kim has posted comments on this change.

Change subject: ASTERIXDB-1088: fixed a secondary index compact issue
..


Patch Set 1: Code-Review+2

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1123
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide9191fce61b767c08d2298070e64d455ee34f0b
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim 
Gerrit-Reviewer: Jenkins 
Gerrit-Reviewer: Young-Seok Kim 
Gerrit-HasComments: No

Change in asterixdb[master]: ASTERIXDB-1538: fix an issue where LSMRTree index instance i...

[Young-Seok Kim (Code Review)]

Young-Seok Kim has posted comments on this change.

Change subject: ASTERIXDB-1538: fix an issue where LSMRTree index instance is 
created during the recovery process.
..


Patch Set 4: Code-Review+2

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1010
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I701a7ad748ff8982534a132a01f5a1ae77c952fd
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim 
Gerrit-Reviewer: Jenkins 
Gerrit-Reviewer: Taewoo Kim 
Gerrit-Reviewer: Young-Seok Kim 
Gerrit-HasComments: No

Change in asterixdb[master]: ASTERIXDB-1538: fix the issue where LSMRTree index instance ...

[Young-Seok Kim (Code Review)]

Young-Seok Kim has posted comments on this change.

Change subject: ASTERIXDB-1538: fix the issue where LSMRTree index instance is 
created during the recovery process.
..


Patch Set 1:

(1 comment)

https://asterix-gerrit.ics.uci.edu/#/c/1010/1/asterixdb/asterix-installer/src/test/resources/transactionts/queries/recover_after_abort/primary_plus_rtree_index_insert_and_delete/primary_plus_rtree_index_insert_and_delete.4.txneu.aql
File 
asterixdb/asterix-installer/src/test/resources/transactionts/queries/recover_after_abort/primary_plus_rtree_index_insert_and_delete/primary_plus_rtree_index_insert_and_delete.4.txneu.aql:

Line 69: /* Finally, all we have should be the rows with the rowid between 50 
and 100 */
Let's remove this last sentence.
This is not guaranteed since while the row 60 is being inserted into a 
partition and concurrently another duplicated key row 61 is inserted into 
another partition and throws duplicated exception before the row 60 is 
committed.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1010
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I701a7ad748ff8982534a132a01f5a1ae77c952fd
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim 
Gerrit-Reviewer: Jenkins 
Gerrit-Reviewer: Young-Seok Kim 
Gerrit-HasComments: Yes

Change in asterixdb[master]: ASTERIXDB-1538: fix the issue where LSMRTree index instance ...

[Young-Seok Kim (Code Review)]

Young-Seok Kim has posted comments on this change.

Change subject: ASTERIXDB-1538: fix the issue where LSMRTree index instance is 
created during the recovery process.
..


Patch Set 2:

..

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1010
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I701a7ad748ff8982534a132a01f5a1ae77c952fd
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim 
Gerrit-Reviewer: Jenkins 
Gerrit-Reviewer: Young-Seok Kim 
Gerrit-HasComments: No

Change in asterixdb[master]: Index-only plan

[Young-Seok Kim (Code Review)]

Young-Seok Kim has posted comments on this change.

Change subject: Index-only plan
..


Patch Set 11:

(21 comments)

The following three issues are discovered.
1. R-tree index on rectangle type is not correctly dealt with for the 
corresponding index-only plan case
   This case should be added as a test case.
2. limit-push-down for primary index scan doesn't seem to work.
3. limit-push-down for primary-index search doesn't seem to work.

Also, please address other comments.

https://asterix-gerrit.ics.uci.edu/#/c/744/11/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractReplicateOperator.java
File 
algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractReplicateOperator.java:

Line 36: private boolean[] outputMaterializationFlags = new 
boolean[outputArity];
I wonder why this flag is required.


https://asterix-gerrit.ics.uci.edu/#/c/744/11/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveRedundantGroupByDecorVarsRule.java
File 
algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveRedundantGroupByDecorVarsRule.java:

Line 45: public class RemoveRedundantGroupByDecorVarsRule implements 
IAlgebraicRewriteRule {
Is this class introduced for just for optimization, not for the correctness 
issue?


https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/PrimaryIndexSearchOperationDatasetLevelCallback.java
File 
asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/PrimaryIndexSearchOperationDatasetLevelCallback.java:

Line 35: public class PrimaryIndexSearchOperationDatasetLevelCallback extends 
AbstractOperationCallback implements ISearchOperationCallback {
This class should be removed.


https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/SecondaryIndexInstantSearchOperationTryLockOnlyOnPKCallback.java
File 
asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/SecondaryIndexInstantSearchOperationTryLockOnlyOnPKCallback.java:

Line 33: public class 
SecondaryIndexInstantSearchOperationTryLockOnlyOnPKCallback extends 
AbstractOperationCallback
Can we give a simpler class name, e.g., 
SecondaryIndexInstantSearchOperationCallback?


https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AbstractIntroduceAccessMethodRule.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AbstractIntroduceAccessMethodRule.java:

Line 644: 
.getIxJoinOuterAdditionalDataSourceVariables(i);
What's the difference between subTreeDataSourceVars and subTreePKs? why is it 
OK for subTreeDataSourceVars to replace some of subTreePks below?


Line 792: expr = (AbstractLogicalExpression) 
childFuncExpr.getArguments().get(0).getValue();
better be dealing with an else case explicitly, i.e., neither Assign nor unnest 
case.


https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodAnalysisContext.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodAnalysisContext.java:

Line 65: //whether a verification (especially for R-Tree case) is 
required after the secondary index search?
what's the difference between 3. requireVerificationAfterSIdxSearch and 4. 
doesSIdxSearchGenerateNoFalsePositiveResults? It seems the same? It would be 
good to explain the difference with example cases.


Line 67: //Does the given index can cover all search predicates?
It seems that the line 67 and 69 should be switched??


https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodJobGenParams.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodJobGenParams.java:

Line 44: protected boolean isPrimaryIndex;
Please say that "isPrimaryIndex is a derived parameter from the index name, so 
this parameter is not included in the 8 (NUM_PARAMS) parameters written and 
read."


https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java:

Line 260: // If it is not granted, then we need to do a secondary index 
lookup, sort PKs,
Do we sort in this case? ( I thought we don't do sort for this trylockfailure 
case?)


Line 1301: 

Change in asterixdb[master]: Files necessary for initial perf regression tests

[Young-Seok Kim (Code Review)]

Young-Seok Kim has posted comments on this change.

Change subject: Files necessary for initial perf regression tests
..


Patch Set 7:

(2 comments)

It will be good to have a high-level document which explains what perf 
regression tests are, what the purpose of the tests is, and what kind of 
queries are executed in the tests. 
There are minor comments.
Please address them.

https://asterix-gerrit.ics.uci.edu/#/c/884/7/asterixdb/asterix-experiments/src/main/java/org/apache/asterix/experiment/builder/AbstractPerfLoadBuilder.java
File 
asterixdb/asterix-experiments/src/main/java/org/apache/asterix/experiment/builder/AbstractPerfLoadBuilder.java:

Line 172: lsAction.add(new 
AbstractRemoteExecutableAction(ncHost, username, sshKeyLocation) {
lsAction seems useless.


Line 194: //load data into pidx
The comment should be something like a "//send SQLPP query"


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/884
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21e2d44ca46a4a28478d9ef256c4fa8d53f03dc9
Gerrit-PatchSet: 7
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon 
Gerrit-Reviewer: Ian Maxon 
Gerrit-Reviewer: Jenkins 
Gerrit-Reviewer: Till Westmann 
Gerrit-Reviewer: Young-Seok Kim 
Gerrit-HasComments: Yes