[GitHub] [phoenix] priyankporwal commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
priyankporwal commented on a change in pull request #683: PHOENIX-5674 
IndexTool to not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366729041
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -504,11 +537,82 @@ public Boolean call() throws Exception {
 });
 }
 
+private void parallelizeIndexVerify() throws IOException {
+addToBeVerifiedIndexRows();
+ArrayList keys = new ArrayList<>(rowCountPerTask);
+Map perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+for (Map.Entry entry: indexKeyToDataPutMap.entrySet()) {
+keys.add(PVarbinary.INSTANCE.getKeyRange(entry.getKey()));
+perTaskDataKeyToDataPutMap.put(entry.getValue().getRow(), 
entry.getValue());
+if (keys.size() == rowCountPerTask) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+keys = new ArrayList<>(rowCountPerTask);
+perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+}
+}
+if (keys.size() > 0) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+}
+List taskResultList = null;
+try {
+LOGGER.debug("Waiting on index verify tasks to complete...");
+taskResultList = this.pool.submitUninterruptible(tasks);
+} catch (ExecutionException e) {
+throw new RuntimeException("Should not fail on the results while 
using a WaitForCompletionTaskRunner", e);
+} catch (EarlyExitFailure e) {
+throw new RuntimeException("Stopped while waiting for batch, 
quitting!", e);
+} finally {
+tasks.getTasks().clear();
+}
+for (Boolean result : taskResultList) {
+if (result == null) {
+// there was a failure
+throw new IOException(exceptionMessage);
+}
+}
+}
+
+private void verifyAndOrRebuildIndex() throws IOException {
+if (verifyType == IndexTool.IndexVerifyType.AFTER || verifyType == 
IndexTool.IndexVerifyType.NONE ||
 
 Review comment:
   Nope .. that's why I said "shortened the names" for my comment purpose.
   Readability suggestion was about organization of the various if() blocks - 
and it sucks how it shows in the comment above.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] abhishek-chouhan commented on issue #685: PHOENIX-5676 Inline-verification from IndexTool does not handle TTL/r…

2020-01-14 Thread GitBox
abhishek-chouhan commented on issue #685: PHOENIX-5676 Inline-verification from 
IndexTool does not handle TTL/r…
URL: https://github.com/apache/phoenix/pull/685#issuecomment-574534652
 
 
   Sorry for that miss. Have updated to use TreeSet with appropriate comparator 
@kadirozde and verified the expected behavior.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to 
not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366727610
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -504,11 +537,82 @@ public Boolean call() throws Exception {
 });
 }
 
+private void parallelizeIndexVerify() throws IOException {
+addToBeVerifiedIndexRows();
+ArrayList keys = new ArrayList<>(rowCountPerTask);
+Map perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+for (Map.Entry entry: indexKeyToDataPutMap.entrySet()) {
+keys.add(PVarbinary.INSTANCE.getKeyRange(entry.getKey()));
+perTaskDataKeyToDataPutMap.put(entry.getValue().getRow(), 
entry.getValue());
+if (keys.size() == rowCountPerTask) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+keys = new ArrayList<>(rowCountPerTask);
+perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+}
+}
+if (keys.size() > 0) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+}
+List taskResultList = null;
+try {
+LOGGER.debug("Waiting on index verify tasks to complete...");
+taskResultList = this.pool.submitUninterruptible(tasks);
+} catch (ExecutionException e) {
+throw new RuntimeException("Should not fail on the results while 
using a WaitForCompletionTaskRunner", e);
+} catch (EarlyExitFailure e) {
+throw new RuntimeException("Stopped while waiting for batch, 
quitting!", e);
+} finally {
+tasks.getTasks().clear();
+}
+for (Boolean result : taskResultList) {
+if (result == null) {
+// there was a failure
+throw new IOException(exceptionMessage);
+}
+}
+}
+
+private void verifyAndOrRebuildIndex() throws IOException {
+if (verifyType == IndexTool.IndexVerifyType.AFTER || verifyType == 
IndexTool.IndexVerifyType.NONE ||
 
 Review comment:
   Is "vt"  more readable than "verifyType"? :-))


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to 
not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366727146
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -504,11 +537,82 @@ public Boolean call() throws Exception {
 });
 }
 
+private void parallelizeIndexVerify() throws IOException {
+addToBeVerifiedIndexRows();
+ArrayList keys = new ArrayList<>(rowCountPerTask);
+Map perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+for (Map.Entry entry: indexKeyToDataPutMap.entrySet()) {
+keys.add(PVarbinary.INSTANCE.getKeyRange(entry.getKey()));
+perTaskDataKeyToDataPutMap.put(entry.getValue().getRow(), 
entry.getValue());
+if (keys.size() == rowCountPerTask) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+keys = new ArrayList<>(rowCountPerTask);
+perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+}
+}
+if (keys.size() > 0) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+}
+List taskResultList = null;
+try {
+LOGGER.debug("Waiting on index verify tasks to complete...");
+taskResultList = this.pool.submitUninterruptible(tasks);
+} catch (ExecutionException e) {
+throw new RuntimeException("Should not fail on the results while 
using a WaitForCompletionTaskRunner", e);
+} catch (EarlyExitFailure e) {
+throw new RuntimeException("Stopped while waiting for batch, 
quitting!", e);
+} finally {
+tasks.getTasks().clear();
+}
+for (Boolean result : taskResultList) {
+if (result == null) {
+// there was a failure
+throw new IOException(exceptionMessage);
+}
+}
+}
+
+private void verifyAndOrRebuildIndex() throws IOException {
+if (verifyType == IndexTool.IndexVerifyType.AFTER || verifyType == 
IndexTool.IndexVerifyType.NONE ||
 
 Review comment:
   Yes, I will remove one of them


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-5645) BaseScannerRegionObserver should prevent compaction from purging very recently deleted cells

2020-01-14 Thread Hadoop QA (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015692#comment-17015692
 ] 

Hadoop QA commented on PHOENIX-5645:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  
http://issues.apache.org/jira/secure/attachment/12990937/PHOENIX-5645-4.14-HBase-1.4.patch
  against 4.14-HBase-1.4 branch at commit 
208509b2ee18699d18bc623d2f09d38b7b298a09.
  ATTACHMENT ID: 12990937

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:green}+1 tests included{color}.  The patch appears to include 4 new 
or modified tests.

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:red}-1 lineLengths{color}.  The patch introduces the following lines 
longer than 100:
+
props.put(QueryServices.GLOBAL_INDEX_ROW_AGE_THRESHOLD_TO_DELETE_MS_ATTRIB, 
Long.toString(0));
+props.put(ScanInfoUtil.PHOENIX_MAX_LOOKBACK_AGE_CONF_KEY, 
Integer.toString(MAX_LOOKBACK_AGE));
+assertRowHasExpectedValueAtSCN(getUrl(), dataTableSelectSql, 
afterInsertSCN, firstValue);
+assertRowHasExpectedValueAtSCN(getUrl(), indexTableSelectSql, 
afterInsertSCN, firstValue);
+while ((lastCompactionTimestamp = 
admin.getLastMajorCompactionTimestamp(table)) < compactionRequestedSCN
+conn.createStatement().execute("upsert into " + tableName + " 
values ('a', 'ab', 'abc', 'abcd')");
+conn.createStatement().execute("upsert into " + tableName + " 
values ('b', 'bc', 'bcd', 'bcde')");
+ String dataTableFullName, String 
indexTableFullName) throws SQLException {
+IndexToolIT.assertExplainPlan(false, actualExplainPlan, 
dataTableFullName, indexTableFullName);
+populateTable(dataTableName); // with two rows ('a', 'ab', 'abc', 
'abcd') and ('b', 'bc', 'bcd', 'bcde')

 {color:red}-1 core tests{color}.  The patch failed these unit tests:
 
./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.IndexToolIT
./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.index.SaltedIndexIT
./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.IndexScrutinyToolIT

Test results: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3302//testReport/
Console output: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3302//console

This message is automatically generated.

> BaseScannerRegionObserver should prevent compaction from purging very 
> recently deleted cells
> 
>
> Key: PHOENIX-5645
> URL: https://issues.apache.org/jira/browse/PHOENIX-5645
> Project: Phoenix
>  Issue Type: Improvement
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Attachments: PHOENIX-5645-4.14-HBase-1.4.patch, 
> PHOENIX-5645-4.x-HBase-1.5-v2.patch, PHOENIX-5645-4.x-HBase-1.5.patch, 
> PHOENIX-5645-4.x-HBase-1.5.v3.patch, PHOENIX-5645-addendum-4.x-HBase-1.5.patch
>
>  Time Spent: 9h 40m
>  Remaining Estimate: 0h
>
> Phoenix's SCN feature has some problems, because HBase major compaction can 
> remove Cells that have been deleted or whose TTL or max versions has caused 
> them to be expired. 
> For example, IndexTool rebuilds and index scrutiny can both give strange, 
> incorrect results if a major compaction occurs in the middle of their run. In 
> the rebuild case, it's because we're rewriting "history" on the index at the 
> same time that compaction is rewriting "history" by purging deleted and 
> expired cells. 
> Create a new configuration property called "max lookback age", which declares 
> that no data written more recently than the max lookback age will be 
> compacted away. The max lookback age must be smaller than the TTL, and it 
> should not be legal for a user to look back further in the past than the 
> table's TTL. 
> Max lookback age by default will not be set, and the current behavior will be 
> preserved. But if max lookback age is set, it will be enforced by the 
> BaseScannerRegionObserver for all tables. 
> In the future, this should be contributed as a general feature to HBase for 
> arbitrary tables. See HBASE-23602.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Hudson (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015679#comment-17015679
 ] 

Hudson commented on PHOENIX-5644:
-

FAILURE: Integrated in Jenkins build PreCommit-PHOENIX-Build #3303 (See 
[https://builds.apache.org/job/PreCommit-PHOENIX-Build/3303/])
PHOENIX-5644 and PHOENIX-5651 addendum patch (s.kadam: rev 
beb6457086c8e51fc8be5eceb468a1edf8a358b7)
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapperForTest.java
* (add) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexUpgradeToolIT.java


> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.master.add.patch, 
> PHOENIX-5644.v1.patch
>
>  Time Spent: 6h 20m
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5678) Cleanup anonymous inner classes used for BaseMutationPlan

2020-01-14 Thread Hadoop QA (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5678?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015678#comment-17015678
 ] 

Hadoop QA commented on PHOENIX-5678:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  
http://issues.apache.org/jira/secure/attachment/12990940/PHOENIX-5678.master.000.patch
  against master branch at commit beb6457086c8e51fc8be5eceb468a1edf8a358b7.
  ATTACHMENT ID: 12990940

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:red}-1 tests included{color}.  The patch doesn't appear to include 
any new or modified tests.
Please justify why no new tests are needed for this 
patch.
Also please list what manual steps were performed to 
verify this patch.

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:green}+1 lineLengths{color}.  The patch does not introduce lines 
longer than 100

{color:green}+1 core tests{color}.  The patch passed unit tests in .

Test results: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3303//testReport/
Console output: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3303//console

This message is automatically generated.

> Cleanup anonymous inner classes used for BaseMutationPlan
> -
>
> Key: PHOENIX-5678
> URL: https://issues.apache.org/jira/browse/PHOENIX-5678
> Project: Phoenix
>  Issue Type: Sub-task
>Affects Versions: 5.1.0, 4.15.1
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
> Fix For: 5.1.0, 4.15.1
>
> Attachments: PHOENIX-5678.master.000.patch
>
>
> BaseMutationPlan has been extended as anonymous inner class at multiple 
> places and some of them have lots of logic placed in overridden methods. We 
> should convert them to Inner classes and use object of extended inner class.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5651) IndexScrutiny does not handle TTL/row-expiry

2020-01-14 Thread Hudson (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015680#comment-17015680
 ] 

Hudson commented on PHOENIX-5651:
-

FAILURE: Integrated in Jenkins build PreCommit-PHOENIX-Build #3303 (See 
[https://builds.apache.org/job/PreCommit-PHOENIX-Build/3303/])
PHOENIX-5644 and PHOENIX-5651 addendum patch (s.kadam: rev 
beb6457086c8e51fc8be5eceb468a1edf8a358b7)
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapperForTest.java
* (add) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexUpgradeToolIT.java


> IndexScrutiny does not handle TTL/row-expiry
> 
>
> Key: PHOENIX-5651
> URL: https://issues.apache.org/jira/browse/PHOENIX-5651
> Project: Phoenix
>  Issue Type: Bug
>Affects Versions: 4.15.1, 4.14.3
>Reporter: Priyank Porwal
>Assignee: Swaroopa Kadam
>Priority: Major
> Fix For: 4.15.1, 4.16.0
>
> Attachments: PHOENIX-5651.4.x-HBase-1.3.patch, 
> PHOENIX-5651.4.x-HBase-1.3.v1.patch, PHOENIX-5651.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5651.master.v1.patch, PHOENIX-5651.master.v2.patch
>
>  Time Spent: 3h 50m
>  Remaining Estimate: 0h
>
> If a data-table has TTL on it, it's indexes inherit the TTL too. Hence when 
> we run IndexScrutiny on such tables and it's indexes, scrutiny's attempts to 
> find matching index rows for near-expiry data rows results in no-matches 
> since the index row gets expired before the read from data-region mapper. The 
> same happens in the MR job for the other direction Index->Data.
> This does not impact correctness of indexing design, but makes it very 
> inconvenient to get a clean scrutiny run. All reported invalid rows have to 
> be matched against the table TTL, which is non-trivial exercise.
> IndexScrutiny itself could detect such expired rows when the matching pair is 
> not found and not report them as INVALID_ROWS. Perhaps a new counter for 
> EXPIRED_ROWS should be added as well for better visibility. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix-queryserver] asfgit closed pull request #16: PHOENIX-5680 remove psql.py from phoenix-queryserver

2020-01-14 Thread GitBox
asfgit closed pull request #16: PHOENIX-5680 remove psql.py from 
phoenix-queryserver
URL: https://github.com/apache/phoenix-queryserver/pull/16
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] priyankporwal commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
priyankporwal commented on a change in pull request #683: PHOENIX-5674 
IndexTool to not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366713145
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -504,11 +537,82 @@ public Boolean call() throws Exception {
 });
 }
 
+private void parallelizeIndexVerify() throws IOException {
+addToBeVerifiedIndexRows();
+ArrayList keys = new ArrayList<>(rowCountPerTask);
+Map perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+for (Map.Entry entry: indexKeyToDataPutMap.entrySet()) {
+keys.add(PVarbinary.INSTANCE.getKeyRange(entry.getKey()));
+perTaskDataKeyToDataPutMap.put(entry.getValue().getRow(), 
entry.getValue());
+if (keys.size() == rowCountPerTask) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+keys = new ArrayList<>(rowCountPerTask);
+perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+}
+}
+if (keys.size() > 0) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+}
+List taskResultList = null;
+try {
+LOGGER.debug("Waiting on index verify tasks to complete...");
+taskResultList = this.pool.submitUninterruptible(tasks);
+} catch (ExecutionException e) {
+throw new RuntimeException("Should not fail on the results while 
using a WaitForCompletionTaskRunner", e);
+} catch (EarlyExitFailure e) {
+throw new RuntimeException("Stopped while waiting for batch, 
quitting!", e);
+} finally {
+tasks.getTasks().clear();
+}
+for (Boolean result : taskResultList) {
+if (result == null) {
+// there was a failure
+throw new IOException(exceptionMessage);
+}
+}
+}
+
+private void verifyAndOrRebuildIndex() throws IOException {
+if (verifyType == IndexTool.IndexVerifyType.AFTER || verifyType == 
IndexTool.IndexVerifyType.NONE ||
 
 Review comment:
   Readability suggestion : (shortened the names)
   
   if (vt == AFTER || vt == NONE) {
   
  if (vt == NONE) {
  return;
  }
   }
   else if (vt == BEFORE || vt == BOTH || vt == ONLY) {
  ...
  if (vt == ONLY)  {
 return;
  }
  
   }
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] priyankporwal commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
priyankporwal commented on a change in pull request #683: PHOENIX-5674 
IndexTool to not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366710379
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -504,11 +537,82 @@ public Boolean call() throws Exception {
 });
 }
 
+private void parallelizeIndexVerify() throws IOException {
+addToBeVerifiedIndexRows();
+ArrayList keys = new ArrayList<>(rowCountPerTask);
+Map perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+for (Map.Entry entry: indexKeyToDataPutMap.entrySet()) {
+keys.add(PVarbinary.INSTANCE.getKeyRange(entry.getKey()));
+perTaskDataKeyToDataPutMap.put(entry.getValue().getRow(), 
entry.getValue());
+if (keys.size() == rowCountPerTask) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+keys = new ArrayList<>(rowCountPerTask);
+perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+}
+}
+if (keys.size() > 0) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+}
+List taskResultList = null;
+try {
+LOGGER.debug("Waiting on index verify tasks to complete...");
+taskResultList = this.pool.submitUninterruptible(tasks);
+} catch (ExecutionException e) {
+throw new RuntimeException("Should not fail on the results while 
using a WaitForCompletionTaskRunner", e);
+} catch (EarlyExitFailure e) {
+throw new RuntimeException("Stopped while waiting for batch, 
quitting!", e);
+} finally {
+tasks.getTasks().clear();
+}
+for (Boolean result : taskResultList) {
+if (result == null) {
+// there was a failure
+throw new IOException(exceptionMessage);
+}
+}
+}
+
+private void verifyAndOrRebuildIndex() throws IOException {
+if (verifyType == IndexTool.IndexVerifyType.AFTER || verifyType == 
IndexTool.IndexVerifyType.NONE ||
 
 Review comment:
   Hmm .. just one of them needs to be removed


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-5656) Make Phoenix scripts work with Python 3

2020-01-14 Thread Lars Hofhansl (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015655#comment-17015655
 ] 

Lars Hofhansl commented on PHOENIX-5656:


-v4 after PHOENIX-5454

> Make Phoenix scripts work with Python 3
> ---
>
> Key: PHOENIX-5656
> URL: https://issues.apache.org/jira/browse/PHOENIX-5656
> Project: Phoenix
>  Issue Type: Bug
>Reporter: Lars Hofhansl
>Priority: Critical
> Fix For: 5.1.0, 4.16.0
>
> Attachments: 5656-4.x-HBase-1.5-untested.txt, 
> 5656-4.x-HBase-1.5-v3.txt, 5656-4.x-HBase-1.5-v4.txt
>
>
> Python 2 is being retired in some environments now. We should make sure that 
> the Phoenix scripts work with Python 2 and 3.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] priyankporwal commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
priyankporwal commented on a change in pull request #683: PHOENIX-5674 
IndexTool to not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366709551
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -504,11 +537,82 @@ public Boolean call() throws Exception {
 });
 }
 
+private void parallelizeIndexVerify() throws IOException {
+addToBeVerifiedIndexRows();
+ArrayList keys = new ArrayList<>(rowCountPerTask);
+Map perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+for (Map.Entry entry: indexKeyToDataPutMap.entrySet()) {
+keys.add(PVarbinary.INSTANCE.getKeyRange(entry.getKey()));
+perTaskDataKeyToDataPutMap.put(entry.getValue().getRow(), 
entry.getValue());
+if (keys.size() == rowCountPerTask) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+keys = new ArrayList<>(rowCountPerTask);
+perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+}
+}
+if (keys.size() > 0) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+}
+List taskResultList = null;
+try {
+LOGGER.debug("Waiting on index verify tasks to complete...");
+taskResultList = this.pool.submitUninterruptible(tasks);
+} catch (ExecutionException e) {
+throw new RuntimeException("Should not fail on the results while 
using a WaitForCompletionTaskRunner", e);
+} catch (EarlyExitFailure e) {
+throw new RuntimeException("Stopped while waiting for batch, 
quitting!", e);
+} finally {
+tasks.getTasks().clear();
+}
+for (Boolean result : taskResultList) {
+if (result == null) {
+// there was a failure
+throw new IOException(exceptionMessage);
+}
+}
+}
+
+private void verifyAndOrRebuildIndex() throws IOException {
+if (verifyType == IndexTool.IndexVerifyType.AFTER || verifyType == 
IndexTool.IndexVerifyType.NONE ||
 
 Review comment:
   2nd and 3rd OR'ed expr are the same. Should the second OR'ed expr be 
verifyType == IndexTool.IndexVerifyType.BOTH?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-5520) Phoenix-level HBase ReplicationEndpoint

2020-01-14 Thread Bharath Vissapragada (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015647#comment-17015647
 ] 

Bharath Vissapragada commented on PHOENIX-5520:
---

I think one solution is to prototype on branch 4.15-HBase-1.5 (since HBase-1.5 
has all the dependencies) and then port it to master. Any better way?

> Phoenix-level HBase ReplicationEndpoint
> ---
>
> Key: PHOENIX-5520
> URL: https://issues.apache.org/jira/browse/PHOENIX-5520
> Project: Phoenix
>  Issue Type: Sub-task
>Reporter: Geoffrey Jacoby
>Priority: Major
>
> A Phoenix implementation of HBase's ReplicationEndpoint that tails the WAL 
> like a normal replication endpoint. However, rather than writing to HBase's 
> replication sink APIs (which create HBase RPCs to a remote cluster), they 
> should write to a new Phoenix Endpoint coprocessor (created in a separate 
> sub-task).
> This assumes that the WAL entries have been annotated with Phoenix metadata 
> (tenant, logical table/view name, timestamp) using the mechanism in 
> PHOENIX-5435.
> While many custom ReplicationEndpoints inherit from 
> HBaseInterClusterReplicationEndpoint and just override the filtering logic, 
> this will need to avoid HBaseInterClusterReplicationEndpoint (which uses 
> HBase RPCs and the HBase sink manager) and instead inherit from 
> BaseReplicationEndpoint, or even implement the ReplicationEndpoint interface 
> + extend AbstractService directly. This is because it has to manage its own 
> transport mechanism to the remote cluster. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to 
not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366699764
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -521,55 +625,58 @@ public boolean next(List results) throws 
IOException {
 if (KeyValue.Type.codeToType(cell.getTypeByte()) 
== KeyValue.Type.Put) {
 if (put == null) {
 put = new Put(CellUtil.cloneRow(cell));
-setMutationAttributes(put, uuidValue);
 mutations.add(put);
 }
 put.add(cell);
 } else {
 if (del == null) {
 del = new Delete(CellUtil.cloneRow(cell));
-setMutationAttributes(del, uuidValue);
 mutations.add(del);
 }
 del.addDeleteMarker(cell);
 }
 }
-if (onlyVerify) {
-rowCount++;
-continue;
+if (scan.isRaw()) {
+if (put != null) {
+setMutationAttributes(put, uuidValue);
+}
+if (del != null) {
+setMutationAttributes(del, uuidValue);
+}
+uuidValue = commitIfReady(uuidValue, mutations);
 }
-uuidValue = commitIfReady(uuidValue);
-if (!scan.isRaw()) {
-Delete deleteMarkers = generateDeleteMarkers(row);
+if (indexRowKey != null) {
+if (put != null) {
+setMutationAttributes(put, uuidValue);
+}
+Delete deleteMarkers = generateDeleteMarkers(put);
 if (deleteMarkers != null) {
 setMutationAttributes(deleteMarkers, 
uuidValue);
 mutations.add(deleteMarkers);
-uuidValue = commitIfReady(uuidValue);
+uuidValue = commitIfReady(uuidValue, 
mutations);
 }
-}
-if (indexRowKey != null) {
 // GlobalIndexChecker passed the index row key. 
This is to build a single index row.
 // Check if the data table row we have just 
scanned matches with the index row key.
 // If not, there is no need to build the index row 
from this data table row,
 // and just return zero row count.
 if (checkIndexRow(indexRowKey, put)) {
 rowCount = 
GlobalIndexChecker.RebuildReturnCode.INDEX_ROW_EXISTS.getValue();
-}
-else {
+} else {
 rowCount = 
GlobalIndexChecker.RebuildReturnCode.NO_INDEX_ROW.getValue();
 }
 break;
 }
 rowCount++;
 }
-
 } while (hasMore && rowCount < pageSizeInRows);
-if (!mutations.isEmpty() && !onlyVerify) {
+}
+if (!scan.isRaw() && indexRowKey == null) {
 
 Review comment:
   I will add comments for it.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to 
not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366698209
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -504,6 +539,75 @@ public Boolean call() throws Exception {
 });
 }
 
+private void parallelizeIndexVerify() throws IOException {
+addToBeVerifiedIndexRows();
+ArrayList keys = new ArrayList<>(rowCountPerTask);
+Map perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+for (Map.Entry entry: indexKeyToDataPutMap.entrySet()) {
+keys.add(PVarbinary.INSTANCE.getKeyRange(entry.getKey()));
+perTaskDataKeyToDataPutMap.put(entry.getValue().getRow(), 
entry.getValue());
+if (keys.size() == rowCountPerTask) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+keys = new ArrayList<>(rowCountPerTask);
+perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+}
+}
+if (keys.size() > 0) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+}
+List taskResultList = null;
+try {
+LOGGER.debug("Waiting on index verify tasks to complete...");
+taskResultList = this.pool.submitUninterruptible(tasks);
+} catch (ExecutionException e) {
+throw new RuntimeException("Should not fail on the results while 
using a WaitForCompletionTaskRunner", e);
+} catch (EarlyExitFailure e) {
+throw new RuntimeException("Stopped while waiting for batch, 
quitting!", e);
+} finally {
+tasks.getTasks().clear();
+}
+for (Boolean result : taskResultList) {
+if (result == null) {
+// there was a failure
+throw new IOException(exceptionMessage);
+}
+}
+}
+
+private void verifyAndOrRebuildIndex() throws IOException {
+if (verifyType == IndexTool.IndexVerifyType.AFTER || verifyType == 
IndexTool.IndexVerifyType.NONE ||
+verifyType == IndexTool.IndexVerifyType.NONE) {
+// For these options we start with rebuilding index rows
+rebuildIndexRows(mutations);
+}
+if (verifyType == IndexTool.IndexVerifyType.NONE) {
+return;
+}
+if (verifyType == IndexTool.IndexVerifyType.BEFORE || verifyType == 
IndexTool.IndexVerifyType.BOTH ||
+verifyType == IndexTool.IndexVerifyType.ONLY) {
+// For these options we start with verifying index rows
+onlyVerify = true; // Don't stop at the first mismatch
+parallelizeIndexVerify();
+}
+if (verifyType == IndexTool.IndexVerifyType.BEFORE || verifyType == 
IndexTool.IndexVerifyType.BOTH) {
+// For these options, we have identified the rows to be rebuilt 
and now need to rebuild them
+// At this point, dataKeyToDataPutMap includes mapping only for 
the rows to be rebuilt
+mutations.clear();
+for (Map.Entry entry: dataKeyToDataPutMap.entrySet()) 
{
+mutations.add(entry.getValue());
+}
+rebuildIndexRows(mutations);
+}
+
+if (verifyType == IndexTool.IndexVerifyType.AFTER || verifyType == 
IndexTool.IndexVerifyType.BOTH) {
+// We have rebuilt index row and now we need to verify them
+onlyVerify = false; // Stop at the first mismatch
 
 Review comment:
   I rename it to doNotFail


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-5520) Phoenix-level HBase ReplicationEndpoint

2020-01-14 Thread Bharath Vissapragada (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015634#comment-17015634
 ] 

Bharath Vissapragada commented on PHOENIX-5520:
---

I've started prototyping this and was attempting to annotate the entries and 
then realized that HBASE-22622 didn't make it to any HBase release in branch-2 
line yet. There has been some recent discussion [1] about cutting branch-2.3 
but I highly doubt if that is going to happen anytime soon. 

Current HBase version dependency of Phoenix core is 2.0.1. What is the ideal 
way to unblock ourselves here? Can the Phoenix master dev branch pull in 
SNAPSHOT dependencies of HBase rather than release versions? How does it work 
usually in cases like these? cc: [~apurtell]

[1] 
https://lists.apache.org/thread.html/2a67c4515338d40c2b81006636d1be0596a40115a6149674bec6ebdc%40%3Cdev.hbase.apache.org%3E

> Phoenix-level HBase ReplicationEndpoint
> ---
>
> Key: PHOENIX-5520
> URL: https://issues.apache.org/jira/browse/PHOENIX-5520
> Project: Phoenix
>  Issue Type: Sub-task
>Reporter: Geoffrey Jacoby
>Priority: Major
>
> A Phoenix implementation of HBase's ReplicationEndpoint that tails the WAL 
> like a normal replication endpoint. However, rather than writing to HBase's 
> replication sink APIs (which create HBase RPCs to a remote cluster), they 
> should write to a new Phoenix Endpoint coprocessor (created in a separate 
> sub-task).
> This assumes that the WAL entries have been annotated with Phoenix metadata 
> (tenant, logical table/view name, timestamp) using the mechanism in 
> PHOENIX-5435.
> While many custom ReplicationEndpoints inherit from 
> HBaseInterClusterReplicationEndpoint and just override the filtering logic, 
> this will need to avoid HBaseInterClusterReplicationEndpoint (which uses 
> HBase RPCs and the HBase sink manager) and instead inherit from 
> BaseReplicationEndpoint, or even implement the ReplicationEndpoint interface 
> + extend AbstractService directly. This is because it has to manage its own 
> transport mechanism to the remote cluster. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to 
not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366697826
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexTool.java
 ##
 @@ -205,11 +205,12 @@ public static IndexVerifyType fromValue(byte[] value) {
 "This parameter is deprecated. Direct mode will be used whether it 
is set or not. Keeping it for backwards compatibility.");
 
 private static final Option VERIFY_OPTION = new Option("v", "verify", true,
-"To verify every data row has a corresponding row. The accepted 
values are NONE, ONLY, BEFORE," +
-" AFTER, and BOTH. NONE is for no inline verification, 
which is also the default for this option. " +
-"ONLY is for verifying without rebuilding index rows. The 
rest for verifying before, after, and " +
-"both before and after rebuilding row. If the verification 
is done before rebuilding rows and " +
-"the correct index rows are not rebuilt. Currently 
supported values are NONE, ONLY and AFTER ");
+"To verify every data row has a corresponding row of a global 
index. For other types of indexes, " +
+"this option will be silently ignored. The accepted values 
are NONE, ONLY, BEFORE,  AFTER, and BOTH. " +
+"NONE is for no inline verification, which is also the 
default for this option. ONLY is for " +
+"verifying without rebuilding index rows. The rest for 
verifying before, after, and both before " +
+"and after rebuilding row. If the verification is done 
before rebuilding rows and the correct " +
 
 Review comment:
   I will reword 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Hadoop QA (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015633#comment-17015633
 ] 

Hadoop QA commented on PHOENIX-5644:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  
http://issues.apache.org/jira/secure/attachment/12990929/PHOENIX-5644.master.add.patch
  against master branch at commit 208509b2ee18699d18bc623d2f09d38b7b298a09.
  ATTACHMENT ID: 12990929

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:green}+1 tests included{color}.  The patch appears to include 2 new 
or modified tests.

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:green}+1 lineLengths{color}.  The patch does not introduce lines 
longer than 100

 {color:red}-1 core tests{color}.  The patch failed these unit tests:
 
./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.PermissionNSEnabledIT

Test results: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3300//testReport/
Console output: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3300//console

This message is automatically generated.

> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.master.add.patch, 
> PHOENIX-5644.v1.patch
>
>  Time Spent: 6h 20m
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to 
not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366697483
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -495,7 +524,13 @@ public Boolean call() throws Exception {
 exceptionMessage = "Pool closed, not attempting to 
verify index rows! " + indexHTable.getName();
 throw new IOException(exceptionMessage);
 }
-verifyIndexRows(keys);
+verifyIndexRows(keys, perTaskRowKeyToDataPutMap);
+if (verifyType == IndexTool.IndexVerifyType.BEFORE || 
verifyType == IndexTool.IndexVerifyType.BOTH) {
+synchronized (dataKeyToDataPutMap) {
 
 Review comment:
   There is no concurrent map for tree maps (i.e., sorted maps). Please note 
that this is called after verifying all rows per task. So, it should not lead 
to a lock contention issue as it will not be a frequent operation.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #683: PHOENIX-5674 IndexTool to 
not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366696867
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -385,13 +388,13 @@ private void verifySingleIndexRow(Result indexRow, final 
Put dataRow) throws IOE
 // This means the index row does not have any covered columns. We 
just need to check if the index row
 // has only one cell (which is the empty column cell)
 if (indexRow.rawCells().length == 1) {
-return;
+return false;
 
 Review comment:
   Yes, this is a success path and it should be true. Good catch.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-5645) BaseScannerRegionObserver should prevent compaction from purging very recently deleted cells

2020-01-14 Thread Hudson (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015621#comment-17015621
 ] 

Hudson commented on PHOENIX-5645:
-

SUCCESS: Integrated in Jenkins build Phoenix-4.x-HBase-1.5 #246 (See 
[https://builds.apache.org/job/Phoenix-4.x-HBase-1.5/246/])
PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from 
(github: rev 164305aad6c0a9a770ad506d92fcf1cbb57e9e20)
* (edit) phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java


> BaseScannerRegionObserver should prevent compaction from purging very 
> recently deleted cells
> 
>
> Key: PHOENIX-5645
> URL: https://issues.apache.org/jira/browse/PHOENIX-5645
> Project: Phoenix
>  Issue Type: Improvement
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Attachments: PHOENIX-5645-4.14-HBase-1.4.patch, 
> PHOENIX-5645-4.x-HBase-1.5-v2.patch, PHOENIX-5645-4.x-HBase-1.5.patch, 
> PHOENIX-5645-4.x-HBase-1.5.v3.patch, PHOENIX-5645-addendum-4.x-HBase-1.5.patch
>
>  Time Spent: 9h 40m
>  Remaining Estimate: 0h
>
> Phoenix's SCN feature has some problems, because HBase major compaction can 
> remove Cells that have been deleted or whose TTL or max versions has caused 
> them to be expired. 
> For example, IndexTool rebuilds and index scrutiny can both give strange, 
> incorrect results if a major compaction occurs in the middle of their run. In 
> the rebuild case, it's because we're rewriting "history" on the index at the 
> same time that compaction is rewriting "history" by purging deleted and 
> expired cells. 
> Create a new configuration property called "max lookback age", which declares 
> that no data written more recently than the max lookback age will be 
> compacted away. The max lookback age must be smaller than the TTL, and it 
> should not be legal for a user to look back further in the past than the 
> table's TTL. 
> Max lookback age by default will not be set, and the current behavior will be 
> preserved. But if max lookback age is set, it will be enforced by the 
> BaseScannerRegionObserver for all tables. 
> In the future, this should be contributed as a general feature to HBase for 
> arbitrary tables. See HBASE-23602.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Hadoop QA (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015619#comment-17015619
 ] 

Hadoop QA commented on PHOENIX-5644:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  
http://issues.apache.org/jira/secure/attachment/12990929/PHOENIX-5644.master.add.patch
  against master branch at commit 208509b2ee18699d18bc623d2f09d38b7b298a09.
  ATTACHMENT ID: 12990929

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:green}+1 tests included{color}.  The patch appears to include 2 new 
or modified tests.

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:green}+1 lineLengths{color}.  The patch does not introduce lines 
longer than 100

{color:green}+1 core tests{color}.  The patch passed unit tests in .

Test results: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3299//testReport/
Console output: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3299//console

This message is automatically generated.

> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.master.add.patch, 
> PHOENIX-5644.v1.patch
>
>  Time Spent: 6h 20m
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] kadirozde commented on a change in pull request #685: PHOENIX-5676 Inline-verification from IndexTool does not handle TTL/r…

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #685: PHOENIX-5676 
Inline-verification from IndexTool does not handle TTL/r…
URL: https://github.com/apache/phoenix/pull/685#discussion_r366688528
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -584,16 +633,19 @@ public boolean next(List results) throws 
IOException {
 if (onlyVerify) {
 addToBeVerifiedIndexRows();
 }
-ArrayList keys = new ArrayList<>(rowCountPerTask);
+ArrayList keyRanges = new ArrayList<>(rowCountPerTask);
+Set indexRowKeys = new HashSet<>(rowCountPerTask);
 
 Review comment:
   A byte array hash set would not work here. The byte array needs to be 
wrapped with equals and hashCode, or Map created with 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR) can be used instead.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-5645) BaseScannerRegionObserver should prevent compaction from purging very recently deleted cells

2020-01-14 Thread Hadoop QA (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015610#comment-17015610
 ] 

Hadoop QA commented on PHOENIX-5645:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  
http://issues.apache.org/jira/secure/attachment/12990935/PHOENIX-5645-addendum-4.x-HBase-1.5.patch
  against 4.x-HBase-1.5 branch at commit 
208509b2ee18699d18bc623d2f09d38b7b298a09.
  ATTACHMENT ID: 12990935

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:green}+0 tests included{color}.  The patch appears to be a 
documentation, build,
or dev patch that doesn't require tests.

{color:red}-1 patch{color}.  The patch command could not apply the patch.

Console output: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3301//console

This message is automatically generated.

> BaseScannerRegionObserver should prevent compaction from purging very 
> recently deleted cells
> 
>
> Key: PHOENIX-5645
> URL: https://issues.apache.org/jira/browse/PHOENIX-5645
> Project: Phoenix
>  Issue Type: Improvement
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Attachments: PHOENIX-5645-4.x-HBase-1.5-v2.patch, 
> PHOENIX-5645-4.x-HBase-1.5.patch, PHOENIX-5645-4.x-HBase-1.5.v3.patch, 
> PHOENIX-5645-addendum-4.x-HBase-1.5.patch
>
>  Time Spent: 9h 40m
>  Remaining Estimate: 0h
>
> Phoenix's SCN feature has some problems, because HBase major compaction can 
> remove Cells that have been deleted or whose TTL or max versions has caused 
> them to be expired. 
> For example, IndexTool rebuilds and index scrutiny can both give strange, 
> incorrect results if a major compaction occurs in the middle of their run. In 
> the rebuild case, it's because we're rewriting "history" on the index at the 
> same time that compaction is rewriting "history" by purging deleted and 
> expired cells. 
> Create a new configuration property called "max lookback age", which declares 
> that no data written more recently than the max lookback age will be 
> compacted away. The max lookback age must be smaller than the TTL, and it 
> should not be legal for a user to look back further in the past than the 
> table's TTL. 
> Max lookback age by default will not be set, and the current behavior will be 
> preserved. But if max lookback age is set, it will be enforced by the 
> BaseScannerRegionObserver for all tables. 
> In the future, this should be contributed as a general feature to HBase for 
> arbitrary tables. See HBASE-23602.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool 
to not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366674366
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -385,13 +388,13 @@ private void verifySingleIndexRow(Result indexRow, final 
Put dataRow) throws IOE
 // This means the index row does not have any covered columns. We 
just need to check if the index row
 // has only one cell (which is the empty column cell)
 if (indexRow.rawCells().length == 1) {
-return;
+return false;
 
 Review comment:
   Shouldn't this return true? (e.g we expected to find only the empty column, 
and we did in fact find only the empty column). The next return false on line 
397 makes sense.  


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool 
to not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366680083
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -504,6 +539,75 @@ public Boolean call() throws Exception {
 });
 }
 
+private void parallelizeIndexVerify() throws IOException {
+addToBeVerifiedIndexRows();
+ArrayList keys = new ArrayList<>(rowCountPerTask);
+Map perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+for (Map.Entry entry: indexKeyToDataPutMap.entrySet()) {
+keys.add(PVarbinary.INSTANCE.getKeyRange(entry.getKey()));
+perTaskDataKeyToDataPutMap.put(entry.getValue().getRow(), 
entry.getValue());
+if (keys.size() == rowCountPerTask) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+keys = new ArrayList<>(rowCountPerTask);
+perTaskDataKeyToDataPutMap = 
Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
+}
+}
+if (keys.size() > 0) {
+addVerifyTask(keys, perTaskDataKeyToDataPutMap);
+}
+List taskResultList = null;
+try {
+LOGGER.debug("Waiting on index verify tasks to complete...");
+taskResultList = this.pool.submitUninterruptible(tasks);
+} catch (ExecutionException e) {
+throw new RuntimeException("Should not fail on the results while 
using a WaitForCompletionTaskRunner", e);
+} catch (EarlyExitFailure e) {
+throw new RuntimeException("Stopped while waiting for batch, 
quitting!", e);
+} finally {
+tasks.getTasks().clear();
+}
+for (Boolean result : taskResultList) {
+if (result == null) {
+// there was a failure
+throw new IOException(exceptionMessage);
+}
+}
+}
+
+private void verifyAndOrRebuildIndex() throws IOException {
+if (verifyType == IndexTool.IndexVerifyType.AFTER || verifyType == 
IndexTool.IndexVerifyType.NONE ||
+verifyType == IndexTool.IndexVerifyType.NONE) {
+// For these options we start with rebuilding index rows
+rebuildIndexRows(mutations);
+}
+if (verifyType == IndexTool.IndexVerifyType.NONE) {
+return;
+}
+if (verifyType == IndexTool.IndexVerifyType.BEFORE || verifyType == 
IndexTool.IndexVerifyType.BOTH ||
+verifyType == IndexTool.IndexVerifyType.ONLY) {
+// For these options we start with verifying index rows
+onlyVerify = true; // Don't stop at the first mismatch
+parallelizeIndexVerify();
+}
+if (verifyType == IndexTool.IndexVerifyType.BEFORE || verifyType == 
IndexTool.IndexVerifyType.BOTH) {
+// For these options, we have identified the rows to be rebuilt 
and now need to rebuild them
+// At this point, dataKeyToDataPutMap includes mapping only for 
the rows to be rebuilt
+mutations.clear();
+for (Map.Entry entry: dataKeyToDataPutMap.entrySet()) 
{
+mutations.add(entry.getValue());
+}
+rebuildIndexRows(mutations);
+}
+
+if (verifyType == IndexTool.IndexVerifyType.AFTER || verifyType == 
IndexTool.IndexVerifyType.BOTH) {
+// We have rebuilt index row and now we need to verify them
+onlyVerify = false; // Stop at the first mismatch
 
 Review comment:
   if onlyVerify now really means "don't stop at first mismatch" whether we're 
doing rebuilds or not, then the variable name is confusing.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool 
to not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366672514
 
 

 ##
 File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolIT.java
 ##
 @@ -408,6 +434,53 @@ private Cell 
getErrorMessageFromIndexToolOutputTable(Connection conn, String dat
 return errorMessageCell;
 }
 
+@Test
+public void testIndexToolVerifyBeforeAndBothOptions() throws Exception {
+// This test is for building non-transactional global indexes with 
direct api
+if (localIndex || transactional || !directApi || useSnapshot) {
+return;
+}
+Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+String schemaName = generateUniqueName();
+String dataTableName = generateUniqueName();
+String dataTableFullName = SchemaUtil.getTableName(schemaName, 
dataTableName);
+String indexTableName = generateUniqueName();
+String viewName = generateUniqueName();
+String viewFullName = SchemaUtil.getTableName(schemaName, 
viewName);
+conn.createStatement().execute("CREATE TABLE " + dataTableFullName
++ " (ID INTEGER NOT NULL PRIMARY KEY, NAME VARCHAR, ZIP 
INTEGER) "
++ tableDDLOptions);
+conn.commit();
+conn.createStatement().execute("CREATE VIEW " + viewFullName + " 
AS SELECT * FROM " + dataTableFullName);
+conn.commit();
+// Insert a row
+conn.createStatement().execute("upsert into " + viewFullName + " 
values (1, 'Phoenix', 12345)");
+conn.commit();
+conn.createStatement().execute(String.format(
+"CREATE INDEX %s ON %s (NAME) INCLUDE (ZIP) ASYNC", 
indexTableName, viewFullName));
+TestUtil.addCoprocessor(conn, "_IDX_" + dataTableFullName, 
MutationCountingRegionObserver.class);
+// Run the index MR job and verify that the index table rebuild 
succeeds
+runIndexTool(directApi, useSnapshot, schemaName, viewName, 
indexTableName,
+null, 0, IndexTool.IndexVerifyType.AFTER);
+assertEquals(1, MutationCountingRegionObserver.getMutationCount());
+MutationCountingRegionObserver.setMutationCount(0);
+// Since all the rows are in the index table, running the index 
tool with the "-v BEFORE" option should
 
 Review comment:
   nit: "should not"


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool 
to not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366675047
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexTool.java
 ##
 @@ -205,11 +205,12 @@ public static IndexVerifyType fromValue(byte[] value) {
 "This parameter is deprecated. Direct mode will be used whether it 
is set or not. Keeping it for backwards compatibility.");
 
 private static final Option VERIFY_OPTION = new Option("v", "verify", true,
-"To verify every data row has a corresponding row. The accepted 
values are NONE, ONLY, BEFORE," +
-" AFTER, and BOTH. NONE is for no inline verification, 
which is also the default for this option. " +
-"ONLY is for verifying without rebuilding index rows. The 
rest for verifying before, after, and " +
-"both before and after rebuilding row. If the verification 
is done before rebuilding rows and " +
-"the correct index rows are not rebuilt. Currently 
supported values are NONE, ONLY and AFTER ");
+"To verify every data row has a corresponding row of a global 
index. For other types of indexes, " +
+"this option will be silently ignored. The accepted values 
are NONE, ONLY, BEFORE,  AFTER, and BOTH. " +
+"NONE is for no inline verification, which is also the 
default for this option. ONLY is for " +
+"verifying without rebuilding index rows. The rest for 
verifying before, after, and both before " +
+"and after rebuilding row. If the verification is done 
before rebuilding rows and the correct " +
 
 Review comment:
   nit: "then the correct index rows will not be rebuilt"


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool 
to not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366681561
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -521,55 +625,58 @@ public boolean next(List results) throws 
IOException {
 if (KeyValue.Type.codeToType(cell.getTypeByte()) 
== KeyValue.Type.Put) {
 if (put == null) {
 put = new Put(CellUtil.cloneRow(cell));
-setMutationAttributes(put, uuidValue);
 mutations.add(put);
 }
 put.add(cell);
 } else {
 if (del == null) {
 del = new Delete(CellUtil.cloneRow(cell));
-setMutationAttributes(del, uuidValue);
 mutations.add(del);
 }
 del.addDeleteMarker(cell);
 }
 }
-if (onlyVerify) {
-rowCount++;
-continue;
+if (scan.isRaw()) {
+if (put != null) {
+setMutationAttributes(put, uuidValue);
+}
+if (del != null) {
+setMutationAttributes(del, uuidValue);
+}
+uuidValue = commitIfReady(uuidValue, mutations);
 }
-uuidValue = commitIfReady(uuidValue);
-if (!scan.isRaw()) {
-Delete deleteMarkers = generateDeleteMarkers(row);
+if (indexRowKey != null) {
+if (put != null) {
+setMutationAttributes(put, uuidValue);
+}
+Delete deleteMarkers = generateDeleteMarkers(put);
 if (deleteMarkers != null) {
 setMutationAttributes(deleteMarkers, 
uuidValue);
 mutations.add(deleteMarkers);
-uuidValue = commitIfReady(uuidValue);
+uuidValue = commitIfReady(uuidValue, 
mutations);
 }
-}
-if (indexRowKey != null) {
 // GlobalIndexChecker passed the index row key. 
This is to build a single index row.
 // Check if the data table row we have just 
scanned matches with the index row key.
 // If not, there is no need to build the index row 
from this data table row,
 // and just return zero row count.
 if (checkIndexRow(indexRowKey, put)) {
 rowCount = 
GlobalIndexChecker.RebuildReturnCode.INDEX_ROW_EXISTS.getValue();
-}
-else {
+} else {
 rowCount = 
GlobalIndexChecker.RebuildReturnCode.NO_INDEX_ROW.getValue();
 }
 break;
 }
 rowCount++;
 }
-
 } while (hasMore && rowCount < pageSizeInRows);
-if (!mutations.isEmpty() && !onlyVerify) {
+}
+if (!scan.isRaw() && indexRowKey == null) {
 
 Review comment:
   what does the scan being raw indicate? A comment would be good. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #683: PHOENIX-5674 IndexTool 
to not write already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683#discussion_r366674451
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
 ##
 @@ -495,7 +524,13 @@ public Boolean call() throws Exception {
 exceptionMessage = "Pool closed, not attempting to 
verify index rows! " + indexHTable.getName();
 throw new IOException(exceptionMessage);
 }
-verifyIndexRows(keys);
+verifyIndexRows(keys, perTaskRowKeyToDataPutMap);
+if (verifyType == IndexTool.IndexVerifyType.BEFORE || 
verifyType == IndexTool.IndexVerifyType.BOTH) {
+synchronized (dataKeyToDataPutMap) {
 
 Review comment:
   nit: would it be better to use a concurrent data structure if we need to 
synchronize access?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] abhishek-chouhan opened a new pull request #685: PHOENIX-5676 Inline-verification from IndexTool does not handle TTL/r…

2020-01-14 Thread GitBox
abhishek-chouhan opened a new pull request #685: PHOENIX-5676 
Inline-verification from IndexTool does not handle TTL/r…
URL: https://github.com/apache/phoenix/pull/685
 
 
   …ow-expiry


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Comment Edited] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Swaroopa Kadam (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015567#comment-17015567
 ] 

Swaroopa Kadam edited comment on PHOENIX-5644 at 1/15/20 2:42 AM:
--

Since the patch didn't apply cleanly on the master, submitting new one for 
master.


was (Author: swaroopa):
Since the patch didn't apply cleanly on the master, submitting new one.

> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.master.add.patch, 
> PHOENIX-5644.v1.patch
>
>  Time Spent: 6h 20m
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] gjacoby126 merged pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 merged pull request #679: PHOENIX-5645 - BaseScannerRegionObserver 
should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366671228
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -105,60 +129,52 @@ public void testTooLowSCNWithMaxLookbackAge() throws 
Exception {
 public void testRecentlyDeletedRowsNotCompactedAway() throws Exception {
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-String fullIndexName = indexStem + "1";
+createTable(dataTableName);
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
 
 Review comment:
   Done.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366671228
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -105,60 +129,52 @@ public void testTooLowSCNWithMaxLookbackAge() throws 
Exception {
 public void testRecentlyDeletedRowsNotCompactedAway() throws Exception {
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-String fullIndexName = indexStem + "1";
+createTable(dataTableName);
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
 
 Review comment:
   Done.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366671327
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -105,60 +129,52 @@ public void testTooLowSCNWithMaxLookbackAge() throws 
Exception {
 public void testRecentlyDeletedRowsNotCompactedAway() throws Exception {
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-String fullIndexName = indexStem + "1";
+createTable(dataTableName);
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
 TableName dataTable = TableName.valueOf(dataTableName);
-TableName indexTable = TableName.valueOf(fullIndexName);
-assertRawRowCount(conn, indexTable, ROWS_POPULATED);
-assertTableHasTtl(conn, indexTable, Integer.MAX_VALUE);
-long beforeDeleteSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
-Thread.sleep(1); //make sure we delete at a different ts
+populateTable(dataTableName);
+//make sure we're after the inserts have been committed
+injectEdge.incValue(1);
+long beforeDeleteSCN = EnvironmentEdgeManager.currentTimeMillis();
+injectEdge.incValue(10); //make sure we delete at a different ts
 Statement stmt = conn.createStatement();
 stmt.execute("DELETE FROM " + dataTableName + " WHERE " + " id = 
'a'");
 Assert.assertEquals(1, stmt.getUpdateCount());
 conn.commit();
 //select stmt to get row we deleted
-String sql = String.format("SELECT * FROM %s WHERE val1 = 'ab'", 
dataTableName);
-assertExplainPlan(conn, sql, dataTableName, fullIndexName);
+String sql = String.format("SELECT * FROM %s WHERE id = 'a'", 
dataTableName);
 int rowsPlusDeleteMarker = ROWS_POPULATED;
-assertRawRowCount(conn, indexTable, rowsPlusDeleteMarker);
 assertRowExistsAtSCN(getUrl(), sql, beforeDeleteSCN, true);
 flush(dataTable);
-flush(indexTable);
-assertRawRowCount(conn, indexTable, rowsPlusDeleteMarker);
 assertRowExistsAtSCN(getUrl(), sql, beforeDeleteSCN, true);
-long beforeFirstCompactSCN = EnvironmentEdgeManager.currentTime();
-Thread.sleep(1);
-majorCompact(indexTable, beforeFirstCompactSCN);
-assertRawRowCount(conn, indexTable, rowsPlusDeleteMarker);
+long beforeFirstCompactSCN = 
EnvironmentEdgeManager.currentTimeMillis();
+injectEdge.incValue(1); //new ts for major compaction
+majorCompact(dataTable, beforeFirstCompactSCN);
+assertRawRowCount(conn, dataTable, rowsPlusDeleteMarker);
 assertRowExistsAtSCN(getUrl(), sql, beforeDeleteSCN, true);
 //wait for the lookback time. After this compactions should purge 
the deleted row
-Thread.sleep(MAX_LOOKBACK_AGE * 1000);
-long beforeSecondCompactSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+injectEdge.incValue(MAX_LOOKBACK_AGE * 1000);
+long beforeSecondCompactSCN = 
EnvironmentEdgeManager.currentTimeMillis();
 String notDeletedRowSql =
-String.format("SELECT * FROM %s WHERE val1 = 'bc'", 
dataTableName);
-assertExplainPlan(conn, notDeletedRowSql, dataTableName, 
fullIndexName);
+String.format("SELECT * FROM %s WHERE id = 'b'", 
dataTableName);
 assertRowExistsAtSCN(getUrl(), notDeletedRowSql, 
beforeSecondCompactSCN, true);
-assertRawRowCount(conn, indexTable, ROWS_POPULATED);
 assertRawRowCount(conn, dataTable, ROWS_POPULATED);
 conn.createStatement().execute("upsert into " + dataTableName +
 " values ('c', 'cd', 'cde', 'cdef')");
 conn.commit();
-majorCompact(indexTable, beforeSecondCompactSCN);
 majorCompact(dataTable, beforeSecondCompactSCN);
 assertRawRowCount(conn, dataTable, ROWS_POPULATED);
 //deleted row should be gone, but not deleted row should still be 
there.
 assertRowExistsAtSCN(getUrl(), sql, beforeSecondCompactSCN, false);
 assertRowExistsAtSCN(getUrl(), notDeletedRowSql, 
beforeSecondCompactSCN, true);
 //1 deleted row should be gone
-assertRawRowCount(conn, indexTable, ROWS_POPULATED);
+assertRawRowCount(conn, dataTable, ROWS_POPULATED);
 }
 }
 
 @Test(timeout=6L)
 
 Review comment:
   And now we're 

[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r36667
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -213,51 +274,57 @@ public void testRecentMaxVersionsNotCompactedAway() 
throws Exception {
 String thirdValue = "ghi";
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem, versions);
-long afterInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+String indexName = generateUniqueName();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
 
 Review comment:
   Thanks, good catch. Fixed.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-4266) Avoid scanner caching in Phoenix

2020-01-14 Thread Andrew Kyle Purtell (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-4266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015575#comment-17015575
 ] 

Andrew Kyle Purtell commented on PHOENIX-4266:
--

Do you mean setCaching or setBatch()? 

Would a patch that simply removes all calls to Scan#setBatch() and setCaching() 
be an acceptable first cut? [~larsh] 

> Avoid scanner caching in Phoenix
> 
>
> Key: PHOENIX-4266
> URL: https://issues.apache.org/jira/browse/PHOENIX-4266
> Project: Phoenix
>  Issue Type: Bug
>Reporter: Lars Hofhansl
>Priority: Major
> Fix For: 5.1.0, 4.16.0
>
>
> Phoenix tries to set caching on all scans. On HBase versions before 0.98 that 
> made sense, now it is the wrong thing to do.
> HBase will by default do size based chunking. Setting scanner caching 
> prevents HBase doing this work.
> We should avoid scanner everywhere, and only use in cases where we know the 
> number of rows to be returned (and that number is small).
> [~sergey.soldatov], [~jamestaylor]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5651) IndexScrutiny does not handle TTL/row-expiry

2020-01-14 Thread Hudson (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015569#comment-17015569
 ] 

Hudson commented on PHOENIX-5651:
-

ABORTED: Integrated in Jenkins build PreCommit-PHOENIX-Build #3298 (See 
[https://builds.apache.org/job/PreCommit-PHOENIX-Build/3298/])
PHOENIX-5651: IndexScrutiny does not handle TTL/row-expiry (#681) (github: rev 
208509b2ee18699d18bc623d2f09d38b7b298a09)
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexScrutinyToolBaseIT.java
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexScrutinyToolIT.java
* (add) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/NonParameterizedIndexScrutinyToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/PhoenixScrutinyJobCounters.java
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexScrutinyToolForTenantIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapper.java
* (add) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapperForTest.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyTool.java


> IndexScrutiny does not handle TTL/row-expiry
> 
>
> Key: PHOENIX-5651
> URL: https://issues.apache.org/jira/browse/PHOENIX-5651
> Project: Phoenix
>  Issue Type: Bug
>Affects Versions: 4.15.1, 4.14.3
>Reporter: Priyank Porwal
>Assignee: Swaroopa Kadam
>Priority: Major
> Fix For: 4.15.1, 4.16.0
>
> Attachments: PHOENIX-5651.4.x-HBase-1.3.patch, 
> PHOENIX-5651.4.x-HBase-1.3.v1.patch, PHOENIX-5651.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5651.master.v1.patch, PHOENIX-5651.master.v2.patch
>
>  Time Spent: 3h 50m
>  Remaining Estimate: 0h
>
> If a data-table has TTL on it, it's indexes inherit the TTL too. Hence when 
> we run IndexScrutiny on such tables and it's indexes, scrutiny's attempts to 
> find matching index rows for near-expiry data rows results in no-matches 
> since the index row gets expired before the read from data-region mapper. The 
> same happens in the MR job for the other direction Index->Data.
> This does not impact correctness of indexing design, but makes it very 
> inconvenient to get a clean scrutiny run. All reported invalid rows have to 
> be matched against the table TTL, which is non-trivial exercise.
> IndexScrutiny itself could detect such expired rows when the matching pair is 
> not found and not report them as INVALID_ROWS. Perhaps a new counter for 
> EXPIRED_ROWS should be added as well for better visibility. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Swaroopa Kadam (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015567#comment-17015567
 ] 

Swaroopa Kadam commented on PHOENIX-5644:
-

Since the patch didn't apply cleanly on the master, submitting new one.

> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.v1.patch
>
>  Time Spent: 6h 20m
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] swaroopak opened a new pull request #684: PHOENIX-5644 and PHOENIX-5651 addendum patch

2020-01-14 Thread GitBox
swaroopak opened a new pull request #684: PHOENIX-5644 and PHOENIX-5651 
addendum patch
URL: https://github.com/apache/phoenix/pull/684
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-5634) Use 'phoenix.default.update.cache.frequency' from connection properties at query time

2020-01-14 Thread Hadoop QA (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5634?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015540#comment-17015540
 ] 

Hadoop QA commented on PHOENIX-5634:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  
http://issues.apache.org/jira/secure/attachment/12990922/PHOENIX-5634.4.x-HBase-1.3.v1.patch
  against 4.x-HBase-1.3 branch at commit 
6405839b60c410292c0e2c8cbbdee55d09343bab.
  ATTACHMENT ID: 12990922

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:red}-1 tests included{color}.  The patch doesn't appear to include 
any new or modified tests.
Please justify why no new tests are needed for this 
patch.
Also please list what manual steps were performed to 
verify this patch.

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:green}+1 lineLengths{color}.  The patch does not introduce lines 
longer than 100

 {color:red}-1 core tests{color}.  The patch failed these unit tests:
 
./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.StatsEnabledSplitSystemCatalogIT
./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.TenantSpecificViewIndexSaltedIT
./phoenix-core/target/failsafe-reports/TEST-org.apache.phoenix.end2end.MaxLookbackIT

Test results: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3297//testReport/
Console output: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3297//console

This message is automatically generated.

> Use 'phoenix.default.update.cache.frequency' from connection properties at 
> query time
> -
>
> Key: PHOENIX-5634
> URL: https://issues.apache.org/jira/browse/PHOENIX-5634
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.15.0, 5.1.0
>Reporter: Nitesh Maheshwari
>Assignee: Nitesh Maheshwari
>Priority: Minor
> Fix For: 5.1.0, 4.15.1
>
> Attachments: PHOENIX-5634.4.x-HBase-1.3.v1.patch, 
> PHOENIX-5634.master.v1.patch
>
>  Time Spent: 4h
>  Remaining Estimate: 0h
>
> We have the config 'phoenix.default.update.cache.frequency' which specifies 
> the time a client should wait before it refreshes its metadata cache entry 
> for a table by fetching the latest metadata from system catalog. This value 
> could be set for a table in the following ways (in the following preference 
> order):
>  # Specifying UPDATE_CACHE_FREQUENCY in table creation DDL
>  # Specifying the connection property 'phoenix.default.update.cache.frequency'
>  # Using the default 'phoenix.default.update.cache.frequency'
> At query time, we look at whether UPDATE_CACHE_FREQUENCY was specified for 
> the table and decide based on that value if the latest metadata for a table 
> should be fetched from system catalog to update the cache. However, when the 
> table doesn't have UPDATE_CACHE_FREQUENCY specified we should look at the 
> connection property 'phoenix.default.update.cache.frequency' (or the default 
> 'phoenix.default.update.cache.frequency' when the connection level property 
> is not set) to make that decision. The support for latter is missing - this 
> Jira is intended to add that.
> This will aid exiting installations where the tables were created without a 
> specified UPDATE_CACHE_FREQUENCY, and thus always hit the system catalog to 
> get the latest metadata when referenced. With this support, we will be able 
> to reduce the load on system catalog by specifying a connection level 
> property for all tables referenced from the connection (as against UPSERTing 
> each table entry in system catalog to set an UPDATE_CACHE_FREQUENCY value).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Comment Edited] (PHOENIX-5265) [UMBRELLA] Phoenix Test should use gold files for result comparison instead of using hard-corded comparison.

2020-01-14 Thread Viraj Jasani (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5265?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015527#comment-17015527
 ] 

Viraj Jasani edited comment on PHOENIX-5265 at 1/15/20 1:16 AM:


{quote}It would also be useful if explain plans had an API that returned an 
object with various fields. Sometimes you just care that the explain picked a 
particular index, or that it's serial vs parallel.
{quote}
[~gjacoby] this should be limited to tests only?


was (Author: vjasani):
{quote}It would also be useful if explain plans had an API that returned an 
object with various fields. Sometimes you just care that the explain picked a 
particular index, or that it's serial vs parallel.
{quote}
[~gjacoby] this should also be limited to tests only right?

> [UMBRELLA] Phoenix Test should use gold files for result comparison instead 
> of using hard-corded comparison.
> 
>
> Key: PHOENIX-5265
> URL: https://issues.apache.org/jira/browse/PHOENIX-5265
> Project: Phoenix
>  Issue Type: Improvement
> Environment: {code:java}
> // code placeholder
> @Test
> public void testWithMultiCF() throws Exception {
> int nRows = 20;
> Connection conn = getConnection(0);
> PreparedStatement stmt;
> conn.createStatement().execute(
> "CREATE TABLE " + fullTableName
> + "(k VARCHAR PRIMARY KEY, a.v INTEGER, b.v INTEGER, c.v 
> INTEGER NULL, d.v INTEGER NULL) "
> + tableDDLOptions );
> stmt = conn.prepareStatement("UPSERT INTO " + fullTableName + " 
> VALUES(?,?, ?, ?, ?)");
> byte[] val = new byte[250];
> for (int i = 0; i < nRows; i++) {
> stmt.setString(1, Character.toString((char)('a' + i)) + 
> Bytes.toString(val));
> stmt.setInt(2, i);
> stmt.setInt(3, i);
> stmt.setInt(4, i);
> stmt.setInt(5, i);
> stmt.executeUpdate();
> }
> conn.commit();
> stmt = conn.prepareStatement("UPSERT INTO " + fullTableName + "(k, c.v, 
> d.v) VALUES(?,?,?)");
> for (int i = 0; i < 5; i++) {
> stmt.setString(1, Character.toString((char)('a' + 'z' + i)) + 
> Bytes.toString(val));
> stmt.setInt(2, i);
> stmt.setInt(3, i);
> stmt.executeUpdate();
> }
> conn.commit();
> ResultSet rs;
> String actualExplainPlan;
> collectStatistics(conn, fullTableName);
> List keyRanges = getAllSplits(conn, fullTableName);
> assertEquals(26, keyRanges.size());
> rs = conn.createStatement().executeQuery("EXPLAIN SELECT * FROM " + 
> fullTableName);
> actualExplainPlan = QueryUtil.getExplainPlan(rs);
> assertEquals(
> "CLIENT 26-CHUNK 25 ROWS " + (columnEncoded ? ( mutable ? "12530" 
> : "14190" ) : 
> (TransactionFactory.Provider.OMID.name().equals(transactionProvider)) ? 
> "25320" : "12420") +
> " BYTES PARALLEL 1-WAY FULL SCAN OVER " + 
> physicalTableName,
> actualExplainPlan);
> ConnectionQueryServices services = 
> conn.unwrap(PhoenixConnection.class).getQueryServices();
> List regions = 
> services.getAllTableRegions(Bytes.toBytes(physicalTableName));
> assertEquals(1, regions.size());
> collectStatistics(conn, fullTableName, Long.toString(1000));
> keyRanges = getAllSplits(conn, fullTableName);
> boolean oneCellPerColFamilyStorageScheme = !mutable && columnEncoded;
> boolean hasShadowCells = 
> TransactionFactory.Provider.OMID.name().equals(transactionProvider);
> assertEquals(oneCellPerColFamilyStorageScheme ? 14 : hasShadowCells ? 24 
> : 13, keyRanges.size());
> rs = conn
> .createStatement()
> .executeQuery(
> "SELECT 
> COLUMN_FAMILY,SUM(GUIDE_POSTS_ROW_COUNT),SUM(GUIDE_POSTS_WIDTH),COUNT(*) from 
> \"SYSTEM\".STATS where PHYSICAL_NAME = '"
> + physicalTableName + "' GROUP BY COLUMN_FAMILY 
> ORDER BY COLUMN_FAMILY");
> assertTrue(rs.next());
> assertEquals("A", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 12530 : 14190 ) : hasShadowCells 
> ? 25320 : 12420, rs.getInt(3));
> assertEquals(oneCellPerColFamilyStorageScheme ? 13 : hasShadowCells ? 23 
> : 12, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("B", rs.getString(1));
> assertEquals(oneCellPerColFamilyStorageScheme ? 25 : 20, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 5600 : 7260 ) : hasShadowCells ? 
> 11260 : 5540, rs.getInt(3));
> assertEquals(oneCellPerColFamilyStorageScheme ? 7 : hasShadowCells ? 10 : 
> 5, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("C", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( 

[GitHub] [phoenix] priyankporwal commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
priyankporwal commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366653511
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -213,51 +274,57 @@ public void testRecentMaxVersionsNotCompactedAway() 
throws Exception {
 String thirdValue = "ghi";
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem, versions);
-long afterInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+String indexName = generateUniqueName();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
 
 Review comment:
   Nit: the comment is not right (nothing here equals 10mins).. wait is only 1 
ms now


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366649341
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
 
 Review comment:
   That's not necessary because we always advance time by MAX_LOOKBACK_AGE 
before trying to look back that far. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366649113
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
+populateTable(dataTableName);
+injectEdge.incValue(1);
+long afterFirstInsertSCN = 
EnvironmentEdgeManager.currentTimeMillis();
 TableName dataTable = TableName.valueOf(dataTableName);
 assertTableHasTtl(conn, dataTable, ttl);
-String fullIndexName = indexStem + "1";
-TableName indexTable = TableName.valueOf(fullIndexName);
-assertTableHasTtl(conn, indexTable, ttl);
-
 //first make sure we inserted correctly
-String sql = String.format("SELECT val2 FROM %s WHERE val1 = 
'ab'", dataTableName);
-assertExplainPlan(conn, sql, dataTableName, fullIndexName);
+String sql = String.format("SELECT val2 FROM %s WHERE id = 'a'", 
dataTableName);
+  //  assertExplainPlan(conn, sql, dataTableName, fullIndexName);
 assertRowExistsAtSCN(getUrl(),sql, afterFirstInsertSCN, true);
 int originalRowCount = 2;
-assertRawRowCount(conn, indexTable, originalRowCount);
+assertRawRowCount(conn, dataTable, originalRowCount);
 //force a flush
-flush(indexTable);
+flush(dataTable);
 //flush shouldn't have changed it
-assertRawRowCount(conn, indexTable, originalRowCount);
+assertRawRowCount(conn, dataTable, originalRowCount);
+  // assertExplainPlan(conn, sql, dataTableName, 
fullIndexName);
 
 Review comment:
   Not commented out anymore. :-)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366649181
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
+populateTable(dataTableName);
+injectEdge.incValue(1);
+long afterFirstInsertSCN = 
EnvironmentEdgeManager.currentTimeMillis();
 TableName dataTable = TableName.valueOf(dataTableName);
 assertTableHasTtl(conn, dataTable, ttl);
-String fullIndexName = indexStem + "1";
-TableName indexTable = TableName.valueOf(fullIndexName);
-assertTableHasTtl(conn, indexTable, ttl);
-
 //first make sure we inserted correctly
-String sql = String.format("SELECT val2 FROM %s WHERE val1 = 
'ab'", dataTableName);
-assertExplainPlan(conn, sql, dataTableName, fullIndexName);
+String sql = String.format("SELECT val2 FROM %s WHERE id = 'a'", 
dataTableName);
+  //  assertExplainPlan(conn, sql, dataTableName, fullIndexName);
 
 Review comment:
   Not commented out anymore. :-)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-5265) [UMBRELLA] Phoenix Test should use gold files for result comparison instead of using hard-corded comparison.

2020-01-14 Thread Viraj Jasani (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5265?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015531#comment-17015531
 ] 

Viraj Jasani commented on PHOENIX-5265:
---

I will create sub-tasks to provide gold files covering specific tests.

> [UMBRELLA] Phoenix Test should use gold files for result comparison instead 
> of using hard-corded comparison.
> 
>
> Key: PHOENIX-5265
> URL: https://issues.apache.org/jira/browse/PHOENIX-5265
> Project: Phoenix
>  Issue Type: Improvement
> Environment: {code:java}
> // code placeholder
> @Test
> public void testWithMultiCF() throws Exception {
> int nRows = 20;
> Connection conn = getConnection(0);
> PreparedStatement stmt;
> conn.createStatement().execute(
> "CREATE TABLE " + fullTableName
> + "(k VARCHAR PRIMARY KEY, a.v INTEGER, b.v INTEGER, c.v 
> INTEGER NULL, d.v INTEGER NULL) "
> + tableDDLOptions );
> stmt = conn.prepareStatement("UPSERT INTO " + fullTableName + " 
> VALUES(?,?, ?, ?, ?)");
> byte[] val = new byte[250];
> for (int i = 0; i < nRows; i++) {
> stmt.setString(1, Character.toString((char)('a' + i)) + 
> Bytes.toString(val));
> stmt.setInt(2, i);
> stmt.setInt(3, i);
> stmt.setInt(4, i);
> stmt.setInt(5, i);
> stmt.executeUpdate();
> }
> conn.commit();
> stmt = conn.prepareStatement("UPSERT INTO " + fullTableName + "(k, c.v, 
> d.v) VALUES(?,?,?)");
> for (int i = 0; i < 5; i++) {
> stmt.setString(1, Character.toString((char)('a' + 'z' + i)) + 
> Bytes.toString(val));
> stmt.setInt(2, i);
> stmt.setInt(3, i);
> stmt.executeUpdate();
> }
> conn.commit();
> ResultSet rs;
> String actualExplainPlan;
> collectStatistics(conn, fullTableName);
> List keyRanges = getAllSplits(conn, fullTableName);
> assertEquals(26, keyRanges.size());
> rs = conn.createStatement().executeQuery("EXPLAIN SELECT * FROM " + 
> fullTableName);
> actualExplainPlan = QueryUtil.getExplainPlan(rs);
> assertEquals(
> "CLIENT 26-CHUNK 25 ROWS " + (columnEncoded ? ( mutable ? "12530" 
> : "14190" ) : 
> (TransactionFactory.Provider.OMID.name().equals(transactionProvider)) ? 
> "25320" : "12420") +
> " BYTES PARALLEL 1-WAY FULL SCAN OVER " + 
> physicalTableName,
> actualExplainPlan);
> ConnectionQueryServices services = 
> conn.unwrap(PhoenixConnection.class).getQueryServices();
> List regions = 
> services.getAllTableRegions(Bytes.toBytes(physicalTableName));
> assertEquals(1, regions.size());
> collectStatistics(conn, fullTableName, Long.toString(1000));
> keyRanges = getAllSplits(conn, fullTableName);
> boolean oneCellPerColFamilyStorageScheme = !mutable && columnEncoded;
> boolean hasShadowCells = 
> TransactionFactory.Provider.OMID.name().equals(transactionProvider);
> assertEquals(oneCellPerColFamilyStorageScheme ? 14 : hasShadowCells ? 24 
> : 13, keyRanges.size());
> rs = conn
> .createStatement()
> .executeQuery(
> "SELECT 
> COLUMN_FAMILY,SUM(GUIDE_POSTS_ROW_COUNT),SUM(GUIDE_POSTS_WIDTH),COUNT(*) from 
> \"SYSTEM\".STATS where PHYSICAL_NAME = '"
> + physicalTableName + "' GROUP BY COLUMN_FAMILY 
> ORDER BY COLUMN_FAMILY");
> assertTrue(rs.next());
> assertEquals("A", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 12530 : 14190 ) : hasShadowCells 
> ? 25320 : 12420, rs.getInt(3));
> assertEquals(oneCellPerColFamilyStorageScheme ? 13 : hasShadowCells ? 23 
> : 12, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("B", rs.getString(1));
> assertEquals(oneCellPerColFamilyStorageScheme ? 25 : 20, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 5600 : 7260 ) : hasShadowCells ? 
> 11260 : 5540, rs.getInt(3));
> assertEquals(oneCellPerColFamilyStorageScheme ? 7 : hasShadowCells ? 10 : 
> 5, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("C", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 7005 : 7280 ) : hasShadowCells ? 
> 14085 : 6930, rs.getInt(3));
> assertEquals(hasShadowCells ? 13 : 7, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("D", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 7005 : 7280 ) : hasShadowCells ? 
> 14085 : 6930, rs.getInt(3));
> assertEquals(hasShadowCells ? 13 : 7, rs.getInt(4));
> assertFalse(rs.next());
> // Disable stats
> conn.createStatement().execute("ALTER TABLE " + fullTableName + 
>

[jira] [Commented] (PHOENIX-5265) [UMBRELLA] Phoenix Test should use gold files for result comparison instead of using hard-corded comparison.

2020-01-14 Thread Viraj Jasani (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5265?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015527#comment-17015527
 ] 

Viraj Jasani commented on PHOENIX-5265:
---

{quote}It would also be useful if explain plans had an API that returned an 
object with various fields. Sometimes you just care that the explain picked a 
particular index, or that it's serial vs parallel.
{quote}
[~gjacoby] this should also be limited to tests only right?

> [UMBRELLA] Phoenix Test should use gold files for result comparison instead 
> of using hard-corded comparison.
> 
>
> Key: PHOENIX-5265
> URL: https://issues.apache.org/jira/browse/PHOENIX-5265
> Project: Phoenix
>  Issue Type: Improvement
> Environment: {code:java}
> // code placeholder
> @Test
> public void testWithMultiCF() throws Exception {
> int nRows = 20;
> Connection conn = getConnection(0);
> PreparedStatement stmt;
> conn.createStatement().execute(
> "CREATE TABLE " + fullTableName
> + "(k VARCHAR PRIMARY KEY, a.v INTEGER, b.v INTEGER, c.v 
> INTEGER NULL, d.v INTEGER NULL) "
> + tableDDLOptions );
> stmt = conn.prepareStatement("UPSERT INTO " + fullTableName + " 
> VALUES(?,?, ?, ?, ?)");
> byte[] val = new byte[250];
> for (int i = 0; i < nRows; i++) {
> stmt.setString(1, Character.toString((char)('a' + i)) + 
> Bytes.toString(val));
> stmt.setInt(2, i);
> stmt.setInt(3, i);
> stmt.setInt(4, i);
> stmt.setInt(5, i);
> stmt.executeUpdate();
> }
> conn.commit();
> stmt = conn.prepareStatement("UPSERT INTO " + fullTableName + "(k, c.v, 
> d.v) VALUES(?,?,?)");
> for (int i = 0; i < 5; i++) {
> stmt.setString(1, Character.toString((char)('a' + 'z' + i)) + 
> Bytes.toString(val));
> stmt.setInt(2, i);
> stmt.setInt(3, i);
> stmt.executeUpdate();
> }
> conn.commit();
> ResultSet rs;
> String actualExplainPlan;
> collectStatistics(conn, fullTableName);
> List keyRanges = getAllSplits(conn, fullTableName);
> assertEquals(26, keyRanges.size());
> rs = conn.createStatement().executeQuery("EXPLAIN SELECT * FROM " + 
> fullTableName);
> actualExplainPlan = QueryUtil.getExplainPlan(rs);
> assertEquals(
> "CLIENT 26-CHUNK 25 ROWS " + (columnEncoded ? ( mutable ? "12530" 
> : "14190" ) : 
> (TransactionFactory.Provider.OMID.name().equals(transactionProvider)) ? 
> "25320" : "12420") +
> " BYTES PARALLEL 1-WAY FULL SCAN OVER " + 
> physicalTableName,
> actualExplainPlan);
> ConnectionQueryServices services = 
> conn.unwrap(PhoenixConnection.class).getQueryServices();
> List regions = 
> services.getAllTableRegions(Bytes.toBytes(physicalTableName));
> assertEquals(1, regions.size());
> collectStatistics(conn, fullTableName, Long.toString(1000));
> keyRanges = getAllSplits(conn, fullTableName);
> boolean oneCellPerColFamilyStorageScheme = !mutable && columnEncoded;
> boolean hasShadowCells = 
> TransactionFactory.Provider.OMID.name().equals(transactionProvider);
> assertEquals(oneCellPerColFamilyStorageScheme ? 14 : hasShadowCells ? 24 
> : 13, keyRanges.size());
> rs = conn
> .createStatement()
> .executeQuery(
> "SELECT 
> COLUMN_FAMILY,SUM(GUIDE_POSTS_ROW_COUNT),SUM(GUIDE_POSTS_WIDTH),COUNT(*) from 
> \"SYSTEM\".STATS where PHYSICAL_NAME = '"
> + physicalTableName + "' GROUP BY COLUMN_FAMILY 
> ORDER BY COLUMN_FAMILY");
> assertTrue(rs.next());
> assertEquals("A", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 12530 : 14190 ) : hasShadowCells 
> ? 25320 : 12420, rs.getInt(3));
> assertEquals(oneCellPerColFamilyStorageScheme ? 13 : hasShadowCells ? 23 
> : 12, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("B", rs.getString(1));
> assertEquals(oneCellPerColFamilyStorageScheme ? 25 : 20, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 5600 : 7260 ) : hasShadowCells ? 
> 11260 : 5540, rs.getInt(3));
> assertEquals(oneCellPerColFamilyStorageScheme ? 7 : hasShadowCells ? 10 : 
> 5, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("C", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 7005 : 7280 ) : hasShadowCells ? 
> 14085 : 6930, rs.getInt(3));
> assertEquals(hasShadowCells ? 13 : 7, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("D", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 7005 : 7280 ) : hasShadowCells ? 
> 14085 : 

[jira] [Commented] (PHOENIX-5265) [UMBRELLA] Phoenix Test should use gold files for result comparison instead of using hard-corded comparison.

2020-01-14 Thread Geoffrey Jacoby (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5265?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015510#comment-17015510
 ] 

Geoffrey Jacoby commented on PHOENIX-5265:
--

It would also be useful if explain plans had an API that returned an object 
with various fields. Sometimes you just care that the explain picked a 
particular index, or that it's serial vs parallel. 

> [UMBRELLA] Phoenix Test should use gold files for result comparison instead 
> of using hard-corded comparison.
> 
>
> Key: PHOENIX-5265
> URL: https://issues.apache.org/jira/browse/PHOENIX-5265
> Project: Phoenix
>  Issue Type: Improvement
> Environment: {code:java}
> // code placeholder
> @Test
> public void testWithMultiCF() throws Exception {
> int nRows = 20;
> Connection conn = getConnection(0);
> PreparedStatement stmt;
> conn.createStatement().execute(
> "CREATE TABLE " + fullTableName
> + "(k VARCHAR PRIMARY KEY, a.v INTEGER, b.v INTEGER, c.v 
> INTEGER NULL, d.v INTEGER NULL) "
> + tableDDLOptions );
> stmt = conn.prepareStatement("UPSERT INTO " + fullTableName + " 
> VALUES(?,?, ?, ?, ?)");
> byte[] val = new byte[250];
> for (int i = 0; i < nRows; i++) {
> stmt.setString(1, Character.toString((char)('a' + i)) + 
> Bytes.toString(val));
> stmt.setInt(2, i);
> stmt.setInt(3, i);
> stmt.setInt(4, i);
> stmt.setInt(5, i);
> stmt.executeUpdate();
> }
> conn.commit();
> stmt = conn.prepareStatement("UPSERT INTO " + fullTableName + "(k, c.v, 
> d.v) VALUES(?,?,?)");
> for (int i = 0; i < 5; i++) {
> stmt.setString(1, Character.toString((char)('a' + 'z' + i)) + 
> Bytes.toString(val));
> stmt.setInt(2, i);
> stmt.setInt(3, i);
> stmt.executeUpdate();
> }
> conn.commit();
> ResultSet rs;
> String actualExplainPlan;
> collectStatistics(conn, fullTableName);
> List keyRanges = getAllSplits(conn, fullTableName);
> assertEquals(26, keyRanges.size());
> rs = conn.createStatement().executeQuery("EXPLAIN SELECT * FROM " + 
> fullTableName);
> actualExplainPlan = QueryUtil.getExplainPlan(rs);
> assertEquals(
> "CLIENT 26-CHUNK 25 ROWS " + (columnEncoded ? ( mutable ? "12530" 
> : "14190" ) : 
> (TransactionFactory.Provider.OMID.name().equals(transactionProvider)) ? 
> "25320" : "12420") +
> " BYTES PARALLEL 1-WAY FULL SCAN OVER " + 
> physicalTableName,
> actualExplainPlan);
> ConnectionQueryServices services = 
> conn.unwrap(PhoenixConnection.class).getQueryServices();
> List regions = 
> services.getAllTableRegions(Bytes.toBytes(physicalTableName));
> assertEquals(1, regions.size());
> collectStatistics(conn, fullTableName, Long.toString(1000));
> keyRanges = getAllSplits(conn, fullTableName);
> boolean oneCellPerColFamilyStorageScheme = !mutable && columnEncoded;
> boolean hasShadowCells = 
> TransactionFactory.Provider.OMID.name().equals(transactionProvider);
> assertEquals(oneCellPerColFamilyStorageScheme ? 14 : hasShadowCells ? 24 
> : 13, keyRanges.size());
> rs = conn
> .createStatement()
> .executeQuery(
> "SELECT 
> COLUMN_FAMILY,SUM(GUIDE_POSTS_ROW_COUNT),SUM(GUIDE_POSTS_WIDTH),COUNT(*) from 
> \"SYSTEM\".STATS where PHYSICAL_NAME = '"
> + physicalTableName + "' GROUP BY COLUMN_FAMILY 
> ORDER BY COLUMN_FAMILY");
> assertTrue(rs.next());
> assertEquals("A", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 12530 : 14190 ) : hasShadowCells 
> ? 25320 : 12420, rs.getInt(3));
> assertEquals(oneCellPerColFamilyStorageScheme ? 13 : hasShadowCells ? 23 
> : 12, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("B", rs.getString(1));
> assertEquals(oneCellPerColFamilyStorageScheme ? 25 : 20, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 5600 : 7260 ) : hasShadowCells ? 
> 11260 : 5540, rs.getInt(3));
> assertEquals(oneCellPerColFamilyStorageScheme ? 7 : hasShadowCells ? 10 : 
> 5, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("C", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 7005 : 7280 ) : hasShadowCells ? 
> 14085 : 6930, rs.getInt(3));
> assertEquals(hasShadowCells ? 13 : 7, rs.getInt(4));
> assertTrue(rs.next());
> assertEquals("D", rs.getString(1));
> assertEquals(25, rs.getInt(2));
> assertEquals(columnEncoded ? ( mutable ? 7005 : 7280 ) : hasShadowCells ? 
> 14085 : 6930, rs.getInt(3));
> assertEquals(hasShadowCells ? 13 : 7, 

[jira] [Commented] (PHOENIX-4845) Support using Row Value Constructors in OFFSET clause for paging in tables where the sort order of PK columns varies

2020-01-14 Thread Daniel Wong (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-4845?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015504#comment-17015504
 ] 

Daniel Wong commented on PHOENIX-4845:
--

Checked the tests that appears to be a environmental hiccup and it ran clean 
locally.  If you have time can [~tdsilva] or [~jamestaylor] take a peek?  Not 
sure who else might have knowledge of the parser/optimizer.

> Support using Row Value Constructors in OFFSET clause for paging in tables 
> where the sort order of PK columns varies
> 
>
> Key: PHOENIX-4845
> URL: https://issues.apache.org/jira/browse/PHOENIX-4845
> Project: Phoenix
>  Issue Type: New Feature
>Reporter: Thomas D'Silva
>Assignee: Daniel Wong
>Priority: Major
>  Labels: DESC, SFDC
> Attachments: PHOENIX-4845-4.x-HBase-1.3.patch, PHOENIX-offset.txt
>
>  Time Spent: 14h 50m
>  Remaining Estimate: 0h
>
> RVCs along with the LIMIT clause are useful for efficiently paging through 
> rows (see [http://phoenix.apache.org/paged.html]). This works well if the pk 
> columns are sorted ascending, we can always use the > operator to query for 
> the next batch of row.
> However if the PK of a table is (A  DESC, B DESC) we cannot use the following 
> query to page through the data
> {code:java}
> SELECT * FROM TABLE WHERE (A, B) > (?, ?) ORDER BY A DESC, B DESC LIMIT 20
> {code}
> Since the rows are sorted by A desc and then by B descending we need change 
> the comparison order
> {code:java}
> SELECT * FROM TABLE WHERE (A, B) < (?, ?) ORDER BY A DESC, B DESC LIMIT 20
> {code}
> If the PK of a table contains columns with mixed sort order for eg (A  DESC, 
> B) then we cannot use RVC to page through data.
> If we supported using RVCs in the offset clause we could use the offset to 
> set the start row of the scan. Clients would not have to have logic to 
> determine the comparison operator. This would also support paging through 
> data for tables where the PK columns are sorted in mixed order.
> {code:java}
> SELECT * FROM TABLE ORDER BY A DESC, B LIMIT 20 OFFSET (?,?)
> {code}
> We would only allow using the offset if the rows are ordered by the sort 
> order of the PK columns of and Index or Primary Table.
> Note that there is some care is needed in the use of OFFSET with indexes.  If 
> the OFFSET is coercible to multiple indexes/base table it could mean very 
> different positions based on key.  To Handle This the INDEX hint needs to be 
> used to specify an index offset for safety.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix-connectors] brfrn169 commented on issue #11: PHOENIX-5619 CREATE TABLE AS SELECT for Phoenix table doesn't work co…

2020-01-14 Thread GitBox
brfrn169 commented on issue #11: PHOENIX-5619 CREATE TABLE AS SELECT for 
Phoenix table doesn't work co…
URL: https://github.com/apache/phoenix-connectors/pull/11#issuecomment-574431615
 
 
   Thank you @joshelser . I'm stuck and can't get a test showing the issue 
actually. Please commit this if you are okay with the patch. I tested it 
locally and it resolved the issue.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366634151
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -213,62 +241,49 @@ public void testRecentMaxVersionsNotCompactedAway() 
throws Exception {
 String thirdValue = "ghi";
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem, versions);
-long afterInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
+populateTable(dataTableName);
+injectEdge.incValue(1); //increment by 1 so we can see our write
+long afterInsertSCN = EnvironmentEdgeManager.currentTimeMillis();
 //make sure table and index metadata is set up right for versions
 TableName dataTable = TableName.valueOf(dataTableName);
 assertTableHasVersions(conn, dataTable, versions);
-String fullIndexName = indexStem + "1";
-TableName indexTable = TableName.valueOf(fullIndexName);
 
 Review comment:
   All right, will do. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] priyankporwal commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
priyankporwal commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366634040
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
 
 Review comment:
   The suggestion was not to have WAIT_AFTER_TABLE_CREATION be a multiple of 
MAX_LOOKBACK_AGE necessarily, but something larger than MAX_LOOKBACK_AGE  -- 
just something so that if you did (now - MAX_LOOKBACK_AGE) it didn't go before 
table creation time.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] kadirozde opened a new pull request #683: PHOENIX-5674 IndexTool to not write already correct index rows/CFs

2020-01-14 Thread GitBox
kadirozde opened a new pull request #683: PHOENIX-5674 IndexTool to not write 
already correct index rows/CFs
URL: https://github.com/apache/phoenix/pull/683
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] swaroopak closed pull request #678: PHOENIX-5644 and PHOENIX-5651 addendum patch

2020-01-14 Thread GitBox
swaroopak closed pull request #678: PHOENIX-5644 and PHOENIX-5651 addendum patch
URL: https://github.com/apache/phoenix/pull/678
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] swaroopak closed pull request #669: PHOENIX-5651: IndexScrutiny does not handle TTL/row-expiry

2020-01-14 Thread GitBox
swaroopak closed pull request #669: PHOENIX-5651: IndexScrutiny does not handle 
TTL/row-expiry
URL: https://github.com/apache/phoenix/pull/669
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] swaroopak opened a new pull request #682: Phoenix 5675

2020-01-14 Thread GitBox
swaroopak opened a new pull request #682: Phoenix 5675
URL: https://github.com/apache/phoenix/pull/682
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #671: PHOENIX-4845 Support using Row Value Constructors in OFFSET clause fo…

2020-01-14 Thread GitBox
ChinmaySKulkarni commented on a change in pull request #671: PHOENIX-4845 
Support using Row Value Constructors in OFFSET clause fo…
URL: https://github.com/apache/phoenix/pull/671#discussion_r366630961
 
 

 ##
 File path: phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java
 ##
 @@ -934,6 +934,22 @@ public static void dumpIndexStatus(Connection conn, 
String indexName) throws IOE
 }
 }
 
+public static void printResultSet(ResultSet rs) throws SQLException {
+while(rs.next()){
+StringBuilder builder = new StringBuilder();
+int columnCount = rs.getMetaData().getColumnCount();
+for(int i = 0; i < columnCount; i++) {
+Object value = rs.getObject(i+1);
+String output = value == null ? "null" : value.toString();
+builder.append(output);
+if(i + 1 < columnCount){
+builder.append(",");
+}
+}
+System.out.println(builder.toString());
 
 Review comment:
   ok disregard.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] dbwong commented on a change in pull request #671: PHOENIX-4845 Support using Row Value Constructors in OFFSET clause fo…

2020-01-14 Thread GitBox
dbwong commented on a change in pull request #671: PHOENIX-4845 Support using 
Row Value Constructors in OFFSET clause fo…
URL: https://github.com/apache/phoenix/pull/671#discussion_r366630326
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java
 ##
 @@ -192,6 +192,15 @@ public static OrderBy compile(StatementContext context,
 }
 compiler.reset();
 }
+
+//If we are not ordered we shouldn't be using RVC Offset
+//I think this makes sense for the pagination case but perhaps we can 
relax this for
+//other use cases.
+//Note If the table is salted we still mark as row ordered in this 
code path
+if(offset.getByteOffset().isPresent() && orderByExpressions.isEmpty()){
+throw new SQLException("Do not allow non ORDER BY with RVC 
OFFSET");
 
 Review comment:
   Will change error message as discussed.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] swaroopak merged pull request #681: PHOENIX-5651_master: IndexScrutiny does not handle TTL/row-expiry

2020-01-14 Thread GitBox
swaroopak merged pull request #681: PHOENIX-5651_master: IndexScrutiny does not 
handle TTL/row-expiry
URL: https://github.com/apache/phoenix/pull/681
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366627476
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
 
 Review comment:
   Turns out any value >= 1 works. I'll just switch it to 1


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] kadirozde commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366627276
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -213,62 +241,49 @@ public void testRecentMaxVersionsNotCompactedAway() 
throws Exception {
 String thirdValue = "ghi";
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem, versions);
-long afterInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
+populateTable(dataTableName);
+injectEdge.incValue(1); //increment by 1 so we can see our write
+long afterInsertSCN = EnvironmentEdgeManager.currentTimeMillis();
 //make sure table and index metadata is set up right for versions
 TableName dataTable = TableName.valueOf(dataTableName);
 assertTableHasVersions(conn, dataTable, versions);
-String fullIndexName = indexStem + "1";
-TableName indexTable = TableName.valueOf(fullIndexName);
 
 Review comment:
   I suggest including indexes back and applying the same operations/checks to 
the index tables along with their data tables.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] dbwong commented on a change in pull request #671: PHOENIX-4845 Support using Row Value Constructors in OFFSET clause fo…

2020-01-14 Thread GitBox
dbwong commented on a change in pull request #671: PHOENIX-4845 Support using 
Row Value Constructors in OFFSET clause fo…
URL: https://github.com/apache/phoenix/pull/671#discussion_r366626790
 
 

 ##
 File path: 
phoenix-core/src/main/java/org/apache/phoenix/compile/OffsetCompiler.java
 ##
 @@ -62,23 +82,233 @@ public SortOrder getSortOrder() {
 
 private OffsetCompiler() {}
 
-public static Integer compile(StatementContext context, 
FilterableStatement statement) throws SQLException {
+// eager initialization
+final private static OffsetCompiler OFFSET_COMPILER = getInstance();
+
+private static OffsetCompiler getInstance() {
+return new OffsetCompiler();
+}
+
+public static OffsetCompiler getOffsetCompiler() {
+return OFFSET_COMPILER;
+}
+
+public CompiledOffset compile(StatementContext context, 
FilterableStatement statement) throws SQLException {
 OffsetNode offsetNode = statement.getOffset();
-if (offsetNode == null) { return null; }
-OffsetParseNodeVisitor visitor = new OffsetParseNodeVisitor(context);
-offsetNode.getOffsetParseNode().accept(visitor);
-return visitor.getOffset();
+if (offsetNode == null) { return new 
CompiledOffset(Optional.absent(), Optional.absent()); }
+if (offsetNode.isIntegerOffset()) {
+OffsetParseNodeVisitor visitor = new 
OffsetParseNodeVisitor(context);
+offsetNode.getOffsetParseNode().accept(visitor);
+Integer offset = visitor.getOffset();
+return new CompiledOffset(Optional.fromNullable(offset), 
Optional.absent());
+} else {
+// We have a RVC offset. See PHOENIX-4845
+
+// This is a EqualParseNode with LHS and RHS 
RowValueConstructorParseNodes
+// This is enforced as part of the grammar
+EqualParseNode equalParseNode = 
(EqualParseNode)offsetNode.getOffsetParseNode();
+
+RowValueConstructorParseNode rvcColumnsParseNode = 
(RowValueConstructorParseNode)equalParseNode.getLHS();
+RowValueConstructorParseNode rvcConstantParseNode = 
(RowValueConstructorParseNode)equalParseNode.getRHS();
+
+// disallow use with aggregations
+if (statement.isAggregate()) { throw new SQLException("RVC Offset 
not allowed in Aggregates"); }
+
+// Get the tables primary keys
+if (context.getResolver().getTables().size() != 1) {
+throw new SQLException("RVC Offset not allowed with zero or 
multiple tables");
+}
+
+PTable pTable = context.getCurrentTable().getTable();
+
+List columns = pTable.getPKColumns();
+
+int numUserColumns = columns.size(); // columns specified by the 
user
+int userColumnIndex = 0; // index into the ordered list, columns, 
of where user specified start
+
+// if we are salted we need to take a subset of the pk
+Integer buckets = pTable.getBucketNum();
+if (buckets != null && buckets > 0) { // We are salted
+numUserColumns--;
+userColumnIndex++;
+}
+
+if (pTable.isMultiTenant() && pTable.getTenantId() != null) {
+// the tenantId is one of the pks and will be handled 
automatically
+numUserColumns--;
+userColumnIndex++;
+}
+
+boolean isIndex = false;
+if (PTableType.INDEX.equals(pTable.getType())) {
+isIndex = true;
+// If we are a view index we have to handle the idxId column
+// Note that viewIndexId comes before tenantId (what about 
salt byte?)
 
 Review comment:
   I added one.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366626438
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
 
 Review comment:
   WAIT_AFTER_TABLE_CREATION isn't meant to be a factor of MAX_LOOKBACK_AGE -- 
it's just an arbitrarily large number to make sure that all the metadata is 
older than the current timestamp. It could probably be lower, but since the 
size doesn't cost us anything I didn't spend too much time trying to optimize 
it. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] swaroopak commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
swaroopak commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366625000
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -105,60 +129,52 @@ public void testTooLowSCNWithMaxLookbackAge() throws 
Exception {
 public void testRecentlyDeletedRowsNotCompactedAway() throws Exception {
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-String fullIndexName = indexStem + "1";
+createTable(dataTableName);
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
 TableName dataTable = TableName.valueOf(dataTableName);
-TableName indexTable = TableName.valueOf(fullIndexName);
-assertRawRowCount(conn, indexTable, ROWS_POPULATED);
-assertTableHasTtl(conn, indexTable, Integer.MAX_VALUE);
-long beforeDeleteSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
-Thread.sleep(1); //make sure we delete at a different ts
+populateTable(dataTableName);
+//make sure we're after the inserts have been committed
+injectEdge.incValue(1);
+long beforeDeleteSCN = EnvironmentEdgeManager.currentTimeMillis();
+injectEdge.incValue(10); //make sure we delete at a different ts
 Statement stmt = conn.createStatement();
 stmt.execute("DELETE FROM " + dataTableName + " WHERE " + " id = 
'a'");
 Assert.assertEquals(1, stmt.getUpdateCount());
 conn.commit();
 //select stmt to get row we deleted
-String sql = String.format("SELECT * FROM %s WHERE val1 = 'ab'", 
dataTableName);
-assertExplainPlan(conn, sql, dataTableName, fullIndexName);
+String sql = String.format("SELECT * FROM %s WHERE id = 'a'", 
dataTableName);
 int rowsPlusDeleteMarker = ROWS_POPULATED;
-assertRawRowCount(conn, indexTable, rowsPlusDeleteMarker);
 assertRowExistsAtSCN(getUrl(), sql, beforeDeleteSCN, true);
 flush(dataTable);
-flush(indexTable);
-assertRawRowCount(conn, indexTable, rowsPlusDeleteMarker);
 assertRowExistsAtSCN(getUrl(), sql, beforeDeleteSCN, true);
-long beforeFirstCompactSCN = EnvironmentEdgeManager.currentTime();
-Thread.sleep(1);
-majorCompact(indexTable, beforeFirstCompactSCN);
-assertRawRowCount(conn, indexTable, rowsPlusDeleteMarker);
+long beforeFirstCompactSCN = 
EnvironmentEdgeManager.currentTimeMillis();
+injectEdge.incValue(1); //new ts for major compaction
+majorCompact(dataTable, beforeFirstCompactSCN);
+assertRawRowCount(conn, dataTable, rowsPlusDeleteMarker);
 assertRowExistsAtSCN(getUrl(), sql, beforeDeleteSCN, true);
 //wait for the lookback time. After this compactions should purge 
the deleted row
-Thread.sleep(MAX_LOOKBACK_AGE * 1000);
-long beforeSecondCompactSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+injectEdge.incValue(MAX_LOOKBACK_AGE * 1000);
+long beforeSecondCompactSCN = 
EnvironmentEdgeManager.currentTimeMillis();
 String notDeletedRowSql =
-String.format("SELECT * FROM %s WHERE val1 = 'bc'", 
dataTableName);
-assertExplainPlan(conn, notDeletedRowSql, dataTableName, 
fullIndexName);
+String.format("SELECT * FROM %s WHERE id = 'b'", 
dataTableName);
 assertRowExistsAtSCN(getUrl(), notDeletedRowSql, 
beforeSecondCompactSCN, true);
-assertRawRowCount(conn, indexTable, ROWS_POPULATED);
 assertRawRowCount(conn, dataTable, ROWS_POPULATED);
 conn.createStatement().execute("upsert into " + dataTableName +
 " values ('c', 'cd', 'cde', 'cdef')");
 conn.commit();
-majorCompact(indexTable, beforeSecondCompactSCN);
 majorCompact(dataTable, beforeSecondCompactSCN);
 assertRawRowCount(conn, dataTable, ROWS_POPULATED);
 //deleted row should be gone, but not deleted row should still be 
there.
 assertRowExistsAtSCN(getUrl(), sql, beforeSecondCompactSCN, false);
 assertRowExistsAtSCN(getUrl(), notDeletedRowSql, 
beforeSecondCompactSCN, true);
 //1 deleted row should be gone
-assertRawRowCount(conn, indexTable, ROWS_POPULATED);
+assertRawRowCount(conn, dataTable, ROWS_POPULATED);
 }
 }
 
 @Test(timeout=6L)
 
 Review comment:
   aah! my bad, I 

[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366625069
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -213,62 +241,49 @@ public void testRecentMaxVersionsNotCompactedAway() 
throws Exception {
 String thirdValue = "ghi";
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem, versions);
-long afterInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
+populateTable(dataTableName);
+injectEdge.incValue(1); //increment by 1 so we can see our write
+long afterInsertSCN = EnvironmentEdgeManager.currentTimeMillis();
 //make sure table and index metadata is set up right for versions
 TableName dataTable = TableName.valueOf(dataTableName);
 assertTableHasVersions(conn, dataTable, versions);
-String fullIndexName = indexStem + "1";
-TableName indexTable = TableName.valueOf(fullIndexName);
 
 Review comment:
   It was originally about index tables, but I eventually realized that this 
needed to apply to all tables in order for SCN to work properly, so there's 
nothing "index specific" about the functionality anymore.
   
   If the consensus is that we should have specific index cases in here to make 
sure nothing in the indexing coprocs break the general behavior, I'll put some 
(back) in. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] swaroopak commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
swaroopak commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366624734
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -105,60 +129,52 @@ public void testTooLowSCNWithMaxLookbackAge() throws 
Exception {
 public void testRecentlyDeletedRowsNotCompactedAway() throws Exception {
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-String fullIndexName = indexStem + "1";
+createTable(dataTableName);
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
 
 Review comment:
   Given that code does not override the method, it makes sense to rename the 
method. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Comment Edited] (PHOENIX-5672) Unable to find cached index metadata with large UPSERT/SELECT and local index.

2020-01-14 Thread Lars Hofhansl (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015368#comment-17015368
 ] 

Lars Hofhansl edited comment on PHOENIX-5672 at 1/14/20 11:17 PM:
--

Btw. I have been disabling this via the setting, and turned off server side 
UPSERT/SELECT, and increased some timeout, and was able to issue 60m row 
upserts on a single server, where previous it would fail around 15m rows.
So again, while it sounds good to do work on the server, it does not scale, and 
it cannot be chunked or paced easily.

In fact it's slow, but quite stable. It works a while and then just finishes. 
That's how things should behave. This is only possible if the server side work 
can be chunked, which is (currently) only possible to do for the client.


was (Author: lhofhansl):
Btw. I have been disabling this via the setting, and turned off server side 
UPSERT/SELECT, and increased some timeout, and was able to issue 60m row 
upserts on a single server, where previous it would fail around 15m rows.
So again, while it sounds good to do work on the server, it does not scale, and 
it cannot be chunked or paced easily.


> Unable to find cached index metadata with large UPSERT/SELECT and local index.
> --
>
> Key: PHOENIX-5672
> URL: https://issues.apache.org/jira/browse/PHOENIX-5672
> Project: Phoenix
>  Issue Type: Bug
>Affects Versions: 4.15.0
>Reporter: Lars Hofhansl
>Priority: Major
>
> Doing a very large UPSERT/SELECT back into the same table. After a while I 
> get this exception. This happens with server side mutation turned off or on 
> and regardless of the batch-size (which I have increased to 1 in this 
> last example).
> {code:java}
> 20/01/10 16:41:54 WARN client.AsyncProcess: #1, table=TEST, attempt=1/35 
> failed=1ops, last exception: 
> org.apache.hadoop.hbase.DoNotRetryIOException: 
> org.apache.hadoop.hbase.DoNotRetryIOException: ERROR 2008 (INT10): ERROR 2008 
> (INT10): Unable to find cached index metadata.  key=-1180967500149768360 
> region=TEST,\x80\x965g\x80\x0F@\xAA\x80Y$\xEF,1578504217187.42467236e0b49fda05fdaaf69de98832.host=lhofhansl-wsl2,16201,157870268
>  Index update failed20/01/10 16:41:54 WARN client.AsyncProcess: #1, 
> table=TEST, attempt=1/35 failed=1ops, last exception: 
> org.apache.hadoop.hbase.DoNotRetryIOException: 
> org.apache.hadoop.hbase.DoNotRetryIOException: ERROR 2008 (INT10): ERROR 2008 
> (INT10): Unable to find cached index metadata.  key=-1180967500149768360 
> region=TEST,\x80\x965g\x80\x0F@\xAA\x80Y$\xEF,1578504217187.42467236e0b49fda05fdaaf69de98832.host=lhofhansl-wsl2,16201,157870268
>  Index update failed at 
> org.apache.phoenix.util.ServerUtil.createIOException(ServerUtil.java:113) at 
> org.apache.phoenix.util.ServerUtil.throwIOException(ServerUtil.java:87) at 
> org.apache.phoenix.index.PhoenixIndexMetaDataBuilder.getIndexMetaDataCache(PhoenixIndexMetaDataBuilder.java:101)
>  at 
> org.apache.phoenix.index.PhoenixIndexMetaDataBuilder.getIndexMetaData(PhoenixIndexMetaDataBuilder.java:51)
>  at 
> org.apache.phoenix.index.PhoenixIndexBuilder.getIndexMetaData(PhoenixIndexBuilder.java:100)
>  at 
> org.apache.phoenix.index.PhoenixIndexBuilder.getIndexMetaData(PhoenixIndexBuilder.java:73)
>  at 
> org.apache.phoenix.hbase.index.builder.IndexBuildManager.getIndexMetaData(IndexBuildManager.java:84)
>  at 
> org.apache.phoenix.hbase.index.IndexRegionObserver.getPhoenixIndexMetaData(IndexRegionObserver.java:594)
>  at 
> org.apache.phoenix.hbase.index.IndexRegionObserver.preBatchMutateWithExceptions(IndexRegionObserver.java:646)
>  at 
> org.apache.phoenix.hbase.index.IndexRegionObserver.preBatchMutate(IndexRegionObserver.java:334)
>  at 
> org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost$35.call(RegionCoprocessorHost.java:1024)
>  at 
> org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost$RegionOperation.call(RegionCoprocessorHost.java:1742)
>  at 
> org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost.execOperation(RegionCoprocessorHost.java:1827)
>  at 
> org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost.execOperation(RegionCoprocessorHost.java:1783)
>  at 
> org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost.preBatchMutate(RegionCoprocessorHost.java:1020)
>  at 
> org.apache.hadoop.hbase.regionserver.HRegion.doMiniBatchMutation(HRegion.java:3425)
>  at 
> org.apache.hadoop.hbase.regionserver.HRegion.batchMutate(HRegion.java:3163) 
> at 
> org.apache.hadoop.hbase.regionserver.HRegion.batchMutate(HRegion.java:3105) 
> at 
> org.apache.hadoop.hbase.regionserver.RSRpcServices.doBatchOp(RSRpcServices.java:944)
>  at 
> org.apache.hadoop.hbase.regionserver.RSRpcServices.doNonAtomicRegionMutation(RSRpcServices.java:872)
>  at 
> 

[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366622629
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -105,60 +129,52 @@ public void testTooLowSCNWithMaxLookbackAge() throws 
Exception {
 public void testRecentlyDeletedRowsNotCompactedAway() throws Exception {
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-String fullIndexName = indexStem + "1";
+createTable(dataTableName);
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
 
 Review comment:
   The ManualEnvironmentEdge is copied from HBase code, where that's the method 
name. I couldn't use the HBase class because Phoenix's EEM is odd and requires 
a specific abstract subclass of Edge


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] priyankporwal commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
priyankporwal commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366622515
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
 
 Review comment:
   Nit: consistent comment here


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] priyankporwal commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
priyankporwal commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366622394
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -59,10 +57,31 @@
 @NeedsOwnMiniClusterTest
 public class MaxLookbackIT extends BaseUniqueNamesOwnClusterIT {
 private static final Log LOG = LogFactory.getLog(MaxLookbackIT.class);
-private static final int MAX_LOOKBACK_AGE = 10;
+private static final int MAX_LOOKBACK_AGE = 15;
 private static final int ROWS_POPULATED = 2;
+public static final int WAIT_AFTER_TABLE_CREATION = 60;
 
 Review comment:
   Suggest setting WAIT_AFTER_TABLE_CREATION = MAX_LOOKBACK_AGE * 100 ... just 
in case someone increases MAX_LOOKBACK_AGE and leaves WAIT_AFTER_TABLE_CREATION 
as is.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] kadirozde commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366622507
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -213,62 +241,49 @@ public void testRecentMaxVersionsNotCompactedAway() 
throws Exception {
 String thirdValue = "ghi";
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem, versions);
-long afterInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
+populateTable(dataTableName);
+injectEdge.incValue(1); //increment by 1 so we can see our write
+long afterInsertSCN = EnvironmentEdgeManager.currentTimeMillis();
 //make sure table and index metadata is set up right for versions
 TableName dataTable = TableName.valueOf(dataTableName);
 assertTableHasVersions(conn, dataTable, versions);
-String fullIndexName = indexStem + "1";
-TableName indexTable = TableName.valueOf(fullIndexName);
 
 Review comment:
   What was the reason for eliminating index tables from the tests? Was not 
this JIRA originally about index tables?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
gjacoby126 commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366622118
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -105,60 +129,52 @@ public void testTooLowSCNWithMaxLookbackAge() throws 
Exception {
 public void testRecentlyDeletedRowsNotCompactedAway() throws Exception {
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-String fullIndexName = indexStem + "1";
+createTable(dataTableName);
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
 TableName dataTable = TableName.valueOf(dataTableName);
-TableName indexTable = TableName.valueOf(fullIndexName);
-assertRawRowCount(conn, indexTable, ROWS_POPULATED);
-assertTableHasTtl(conn, indexTable, Integer.MAX_VALUE);
-long beforeDeleteSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
-Thread.sleep(1); //make sure we delete at a different ts
+populateTable(dataTableName);
+//make sure we're after the inserts have been committed
+injectEdge.incValue(1);
+long beforeDeleteSCN = EnvironmentEdgeManager.currentTimeMillis();
+injectEdge.incValue(10); //make sure we delete at a different ts
 Statement stmt = conn.createStatement();
 stmt.execute("DELETE FROM " + dataTableName + " WHERE " + " id = 
'a'");
 Assert.assertEquals(1, stmt.getUpdateCount());
 conn.commit();
 //select stmt to get row we deleted
-String sql = String.format("SELECT * FROM %s WHERE val1 = 'ab'", 
dataTableName);
-assertExplainPlan(conn, sql, dataTableName, fullIndexName);
+String sql = String.format("SELECT * FROM %s WHERE id = 'a'", 
dataTableName);
 int rowsPlusDeleteMarker = ROWS_POPULATED;
-assertRawRowCount(conn, indexTable, rowsPlusDeleteMarker);
 assertRowExistsAtSCN(getUrl(), sql, beforeDeleteSCN, true);
 flush(dataTable);
-flush(indexTable);
-assertRawRowCount(conn, indexTable, rowsPlusDeleteMarker);
 assertRowExistsAtSCN(getUrl(), sql, beforeDeleteSCN, true);
-long beforeFirstCompactSCN = EnvironmentEdgeManager.currentTime();
-Thread.sleep(1);
-majorCompact(indexTable, beforeFirstCompactSCN);
-assertRawRowCount(conn, indexTable, rowsPlusDeleteMarker);
+long beforeFirstCompactSCN = 
EnvironmentEdgeManager.currentTimeMillis();
+injectEdge.incValue(1); //new ts for major compaction
+majorCompact(dataTable, beforeFirstCompactSCN);
+assertRawRowCount(conn, dataTable, rowsPlusDeleteMarker);
 assertRowExistsAtSCN(getUrl(), sql, beforeDeleteSCN, true);
 //wait for the lookback time. After this compactions should purge 
the deleted row
-Thread.sleep(MAX_LOOKBACK_AGE * 1000);
-long beforeSecondCompactSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+injectEdge.incValue(MAX_LOOKBACK_AGE * 1000);
+long beforeSecondCompactSCN = 
EnvironmentEdgeManager.currentTimeMillis();
 String notDeletedRowSql =
-String.format("SELECT * FROM %s WHERE val1 = 'bc'", 
dataTableName);
-assertExplainPlan(conn, notDeletedRowSql, dataTableName, 
fullIndexName);
+String.format("SELECT * FROM %s WHERE id = 'b'", 
dataTableName);
 assertRowExistsAtSCN(getUrl(), notDeletedRowSql, 
beforeSecondCompactSCN, true);
-assertRawRowCount(conn, indexTable, ROWS_POPULATED);
 assertRawRowCount(conn, dataTable, ROWS_POPULATED);
 conn.createStatement().execute("upsert into " + dataTableName +
 " values ('c', 'cd', 'cde', 'cdef')");
 conn.commit();
-majorCompact(indexTable, beforeSecondCompactSCN);
 majorCompact(dataTable, beforeSecondCompactSCN);
 assertRawRowCount(conn, dataTable, ROWS_POPULATED);
 //deleted row should be gone, but not deleted row should still be 
there.
 assertRowExistsAtSCN(getUrl(), sql, beforeSecondCompactSCN, false);
 assertRowExistsAtSCN(getUrl(), notDeletedRowSql, 
beforeSecondCompactSCN, true);
 //1 deleted row should be gone
-assertRawRowCount(conn, indexTable, ROWS_POPULATED);
+assertRawRowCount(conn, dataTable, ROWS_POPULATED);
 }
 }
 
 @Test(timeout=6L)
 
 Review comment:
   Not sure exactly 

[jira] [Comment Edited] (PHOENIX-5645) BaseScannerRegionObserver should prevent compaction from purging very recently deleted cells

2020-01-14 Thread Lars Hofhansl (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015474#comment-17015474
 ] 

Lars Hofhansl edited comment on PHOENIX-5645 at 1/14/20 11:11 PM:
--

Looks like MaxLookbackIT is failing. It went unnoticed because the build 
aborted for a while due to PHOENIX-5644.
Please have a look. I'm also happy to revert and then we re-apply the change 
together with a test fix.


was (Author: lhofhansl):
This is breaking MaxLookbackIT. It went unnoticed because the build aborted for 
a while due to PHOENIX-5644.
Please have a look. I'm also happy to revert and then we re-apply the change 
together with a test fix.

> BaseScannerRegionObserver should prevent compaction from purging very 
> recently deleted cells
> 
>
> Key: PHOENIX-5645
> URL: https://issues.apache.org/jira/browse/PHOENIX-5645
> Project: Phoenix
>  Issue Type: Improvement
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Attachments: PHOENIX-5645-4.x-HBase-1.5-v2.patch, 
> PHOENIX-5645-4.x-HBase-1.5.patch, PHOENIX-5645-4.x-HBase-1.5.v3.patch
>
>  Time Spent: 6h 10m
>  Remaining Estimate: 0h
>
> Phoenix's SCN feature has some problems, because HBase major compaction can 
> remove Cells that have been deleted or whose TTL or max versions has caused 
> them to be expired. 
> For example, IndexTool rebuilds and index scrutiny can both give strange, 
> incorrect results if a major compaction occurs in the middle of their run. In 
> the rebuild case, it's because we're rewriting "history" on the index at the 
> same time that compaction is rewriting "history" by purging deleted and 
> expired cells. 
> Create a new configuration property called "max lookback age", which declares 
> that no data written more recently than the max lookback age will be 
> compacted away. The max lookback age must be smaller than the TTL, and it 
> should not be legal for a user to look back further in the past than the 
> table's TTL. 
> Max lookback age by default will not be set, and the current behavior will be 
> preserved. But if max lookback age is set, it will be enforced by the 
> BaseScannerRegionObserver for all tables. 
> In the future, this should be contributed as a general feature to HBase for 
> arbitrary tables. See HBASE-23602.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5645) BaseScannerRegionObserver should prevent compaction from purging very recently deleted cells

2020-01-14 Thread Lars Hofhansl (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015474#comment-17015474
 ] 

Lars Hofhansl commented on PHOENIX-5645:


This is breaking MaxLookbackIT. It went unnoticed because the build aborted for 
a while due to PHOENIX-5644.
Please have a look. I'm also happy to revert and then we re-apply the change 
together with a test fix.

> BaseScannerRegionObserver should prevent compaction from purging very 
> recently deleted cells
> 
>
> Key: PHOENIX-5645
> URL: https://issues.apache.org/jira/browse/PHOENIX-5645
> Project: Phoenix
>  Issue Type: Improvement
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Attachments: PHOENIX-5645-4.x-HBase-1.5-v2.patch, 
> PHOENIX-5645-4.x-HBase-1.5.patch, PHOENIX-5645-4.x-HBase-1.5.v3.patch
>
>  Time Spent: 6h
>  Remaining Estimate: 0h
>
> Phoenix's SCN feature has some problems, because HBase major compaction can 
> remove Cells that have been deleted or whose TTL or max versions has caused 
> them to be expired. 
> For example, IndexTool rebuilds and index scrutiny can both give strange, 
> incorrect results if a major compaction occurs in the middle of their run. In 
> the rebuild case, it's because we're rewriting "history" on the index at the 
> same time that compaction is rewriting "history" by purging deleted and 
> expired cells. 
> Create a new configuration property called "max lookback age", which declares 
> that no data written more recently than the max lookback age will be 
> compacted away. The max lookback age must be smaller than the TTL, and it 
> should not be legal for a user to look back further in the past than the 
> table's TTL. 
> Max lookback age by default will not be set, and the current behavior will be 
> preserved. But if max lookback age is set, it will be enforced by the 
> BaseScannerRegionObserver for all tables. 
> In the future, this should be contributed as a general feature to HBase for 
> arbitrary tables. See HBASE-23602.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] swaroopak commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
swaroopak commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366621634
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -105,60 +129,52 @@ public void testTooLowSCNWithMaxLookbackAge() throws 
Exception {
 public void testRecentlyDeletedRowsNotCompactedAway() throws Exception {
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-String fullIndexName = indexStem + "1";
+createTable(dataTableName);
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
 
 Review comment:
   nit: increamentValue


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] kadirozde commented on a change in pull request #679: PHOENIX-5645 - BaseScannerRegionObserver should prevent compaction from purg…

2020-01-14 Thread GitBox
kadirozde commented on a change in pull request #679: PHOENIX-5645 - 
BaseScannerRegionObserver should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366620077
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
+populateTable(dataTableName);
+injectEdge.incValue(1);
+long afterFirstInsertSCN = 
EnvironmentEdgeManager.currentTimeMillis();
 TableName dataTable = TableName.valueOf(dataTableName);
 assertTableHasTtl(conn, dataTable, ttl);
-String fullIndexName = indexStem + "1";
-TableName indexTable = TableName.valueOf(fullIndexName);
-assertTableHasTtl(conn, indexTable, ttl);
-
 //first make sure we inserted correctly
-String sql = String.format("SELECT val2 FROM %s WHERE val1 = 
'ab'", dataTableName);
-assertExplainPlan(conn, sql, dataTableName, fullIndexName);
+String sql = String.format("SELECT val2 FROM %s WHERE id = 'a'", 
dataTableName);
+  //  assertExplainPlan(conn, sql, dataTableName, fullIndexName);
 assertRowExistsAtSCN(getUrl(),sql, afterFirstInsertSCN, true);
 int originalRowCount = 2;
-assertRawRowCount(conn, indexTable, originalRowCount);
+assertRawRowCount(conn, dataTable, originalRowCount);
 //force a flush
-flush(indexTable);
+flush(dataTable);
 //flush shouldn't have changed it
-assertRawRowCount(conn, indexTable, originalRowCount);
+assertRawRowCount(conn, dataTable, originalRowCount);
+  // assertExplainPlan(conn, sql, dataTableName, 
fullIndexName);
+long timeToSleep = (MAX_LOOKBACK_AGE * 1000) -
+(EnvironmentEdgeManager.currentTimeMillis() - 
afterFirstInsertSCN);
+if (timeToSleep > 0) {
+injectEdge.incValue(timeToSleep);
+//Thread.sleep(timeToSleep);
+}
+//make sure it's still on disk
+assertRawRowCount(conn, dataTable, originalRowCount);
+injectEdge.incValue(1); //get a new timestamp for compaction
+majorCompact(dataTable, 
EnvironmentEdgeManager.currentTimeMillis());
+//nothing should have been purged by this major compaction
+assertRawRowCount(conn, dataTable, originalRowCount);
 //now wait the TTL
-Thread.sleep((ttl +1) * 1000);
-long afterTTLExpiresSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
-assertExplainPlan(conn, sql, dataTableName, fullIndexName);
-//make sure we can't see it after expiration from masking
-assertRowExistsAtSCN(getUrl(), sql, afterTTLExpiresSCN, false);
-//but it's still on disk
-assertRawRowCount(conn, indexTable, originalRowCount);
-long beforeMajorCompactSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
-majorCompact(indexTable, beforeMajorCompactSCN);
-assertRawRowCount(conn, indexTable, 0);
+timeToSleep = (ttl * 1000) -
 
 Review comment:
   nit : timeToAdvance sounds better than timeToSleep to me


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Comment Edited] (PHOENIX-5678) Cleanup anonymous inner classes used for BaseMutationPlan

2020-01-14 Thread Viraj Jasani (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5678?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015437#comment-17015437
 ] 

Viraj Jasani edited comment on PHOENIX-5678 at 1/14/20 10:12 PM:
-

The build result([https://builds.apache.org/job/PreCommit-PHOENIX-Build/3286/]):
{code:java}
[WARNING] Tests run: 78, Failures: 0, Errors: 0, Skipped: 12, Time elapsed: 
431.465 s - in org.apache.phoenix.schema.stats.TxStatsCollectorIT
Build timed out (after 300 minutes). Marking the build as aborted.
Build was aborted
Archiving artifacts
[INFO] 
[INFO] Results:
[INFO] 
[WARNING] Tests run: 972, Failures: 0, Errors: 0, Skipped: 59
{code}


was (Author: vjasani):
The build 
[result|[https://builds.apache.org/job/PreCommit-PHOENIX-Build/3286/]]:
{code:java}
[WARNING] Tests run: 78, Failures: 0, Errors: 0, Skipped: 12, Time elapsed: 
431.465 s - in org.apache.phoenix.schema.stats.TxStatsCollectorIT
Build timed out (after 300 minutes). Marking the build as aborted.
Build was aborted
Archiving artifacts
[INFO] 
[INFO] Results:
[INFO] 
[WARNING] Tests run: 972, Failures: 0, Errors: 0, Skipped: 59
{code}

> Cleanup anonymous inner classes used for BaseMutationPlan
> -
>
> Key: PHOENIX-5678
> URL: https://issues.apache.org/jira/browse/PHOENIX-5678
> Project: Phoenix
>  Issue Type: Sub-task
>Affects Versions: 5.1.0, 4.15.1
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
> Fix For: 5.1.0, 4.15.1
>
> Attachments: PHOENIX-5678.master.000.patch
>
>
> BaseMutationPlan has been extended as anonymous inner class at multiple 
> places and some of them have lots of logic placed in overridden methods. We 
> should convert them to Inner classes and use object of extended inner class.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] gokceni commented on issue #679: PHOENIX-5645 - GlobalIndexChecker should prevent compaction from purg…

2020-01-14 Thread GitBox
gokceni commented on issue #679: PHOENIX-5645 - GlobalIndexChecker should 
prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#issuecomment-574393825
 
 
   Couple of nits. Don't we want  a test on index tables as well?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gokceni commented on a change in pull request #679: PHOENIX-5645 - GlobalIndexChecker should prevent compaction from purg…

2020-01-14 Thread GitBox
gokceni commented on a change in pull request #679: PHOENIX-5645 - 
GlobalIndexChecker should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366593957
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
+populateTable(dataTableName);
+injectEdge.incValue(1);
+long afterFirstInsertSCN = 
EnvironmentEdgeManager.currentTimeMillis();
 TableName dataTable = TableName.valueOf(dataTableName);
 assertTableHasTtl(conn, dataTable, ttl);
-String fullIndexName = indexStem + "1";
-TableName indexTable = TableName.valueOf(fullIndexName);
-assertTableHasTtl(conn, indexTable, ttl);
-
 //first make sure we inserted correctly
-String sql = String.format("SELECT val2 FROM %s WHERE val1 = 
'ab'", dataTableName);
-assertExplainPlan(conn, sql, dataTableName, fullIndexName);
+String sql = String.format("SELECT val2 FROM %s WHERE id = 'a'", 
dataTableName);
+  //  assertExplainPlan(conn, sql, dataTableName, fullIndexName);
 assertRowExistsAtSCN(getUrl(),sql, afterFirstInsertSCN, true);
 int originalRowCount = 2;
-assertRawRowCount(conn, indexTable, originalRowCount);
+assertRawRowCount(conn, dataTable, originalRowCount);
 //force a flush
-flush(indexTable);
+flush(dataTable);
 //flush shouldn't have changed it
-assertRawRowCount(conn, indexTable, originalRowCount);
+assertRawRowCount(conn, dataTable, originalRowCount);
+  // assertExplainPlan(conn, sql, dataTableName, 
fullIndexName);
 
 Review comment:
   nit: remove commented code


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gokceni commented on a change in pull request #679: PHOENIX-5645 - GlobalIndexChecker should prevent compaction from purg…

2020-01-14 Thread GitBox
gokceni commented on a change in pull request #679: PHOENIX-5645 - 
GlobalIndexChecker should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366594061
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
+populateTable(dataTableName);
+injectEdge.incValue(1);
+long afterFirstInsertSCN = 
EnvironmentEdgeManager.currentTimeMillis();
 TableName dataTable = TableName.valueOf(dataTableName);
 assertTableHasTtl(conn, dataTable, ttl);
-String fullIndexName = indexStem + "1";
-TableName indexTable = TableName.valueOf(fullIndexName);
-assertTableHasTtl(conn, indexTable, ttl);
-
 //first make sure we inserted correctly
-String sql = String.format("SELECT val2 FROM %s WHERE val1 = 
'ab'", dataTableName);
-assertExplainPlan(conn, sql, dataTableName, fullIndexName);
+String sql = String.format("SELECT val2 FROM %s WHERE id = 'a'", 
dataTableName);
+  //  assertExplainPlan(conn, sql, dataTableName, fullIndexName);
 assertRowExistsAtSCN(getUrl(),sql, afterFirstInsertSCN, true);
 int originalRowCount = 2;
-assertRawRowCount(conn, indexTable, originalRowCount);
+assertRawRowCount(conn, dataTable, originalRowCount);
 //force a flush
-flush(indexTable);
+flush(dataTable);
 //flush shouldn't have changed it
-assertRawRowCount(conn, indexTable, originalRowCount);
+assertRawRowCount(conn, dataTable, originalRowCount);
+  // assertExplainPlan(conn, sql, dataTableName, 
fullIndexName);
+long timeToSleep = (MAX_LOOKBACK_AGE * 1000) -
+(EnvironmentEdgeManager.currentTimeMillis() - 
afterFirstInsertSCN);
+if (timeToSleep > 0) {
+injectEdge.incValue(timeToSleep);
+//Thread.sleep(timeToSleep);
 
 Review comment:
   nit: remove commented code


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gokceni commented on a change in pull request #679: PHOENIX-5645 - GlobalIndexChecker should prevent compaction from purg…

2020-01-14 Thread GitBox
gokceni commented on a change in pull request #679: PHOENIX-5645 - 
GlobalIndexChecker should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366589942
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -168,36 +184,48 @@ public void testTTLAndMaxLookbackAge() throws Exception {
 conf.setLong(HRegion.MEMSTORE_PERIODIC_FLUSH_INTERVAL, 0L);
 try (Connection conn = DriverManager.getConnection(getUrl())) {
 String dataTableName = generateUniqueName();
-String indexStem = generateUniqueName();
-createTableAndIndexes(conn, dataTableName, indexStem);
-long afterFirstInsertSCN = 
org.apache.phoenix.util.EnvironmentEdgeManager.currentTimeMillis();
+createTable(dataTableName);
+//increment by 10 min to make sure we don't "look back" past table 
creation
+injectEdge.incValue(WAIT_AFTER_TABLE_CREATION);
+populateTable(dataTableName);
+injectEdge.incValue(1);
+long afterFirstInsertSCN = 
EnvironmentEdgeManager.currentTimeMillis();
 TableName dataTable = TableName.valueOf(dataTableName);
 assertTableHasTtl(conn, dataTable, ttl);
-String fullIndexName = indexStem + "1";
-TableName indexTable = TableName.valueOf(fullIndexName);
-assertTableHasTtl(conn, indexTable, ttl);
-
 //first make sure we inserted correctly
-String sql = String.format("SELECT val2 FROM %s WHERE val1 = 
'ab'", dataTableName);
-assertExplainPlan(conn, sql, dataTableName, fullIndexName);
+String sql = String.format("SELECT val2 FROM %s WHERE id = 'a'", 
dataTableName);
+  //  assertExplainPlan(conn, sql, dataTableName, fullIndexName);
 
 Review comment:
   nit: remove commented code


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] gokceni commented on a change in pull request #679: PHOENIX-5645 - GlobalIndexChecker should prevent compaction from purg…

2020-01-14 Thread GitBox
gokceni commented on a change in pull request #679: PHOENIX-5645 - 
GlobalIndexChecker should prevent compaction from purg…
URL: https://github.com/apache/phoenix/pull/679#discussion_r366589265
 
 

 ##
 File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java
 ##
 @@ -59,10 +57,31 @@
 @NeedsOwnMiniClusterTest
 public class MaxLookbackIT extends BaseUniqueNamesOwnClusterIT {
 private static final Log LOG = LogFactory.getLog(MaxLookbackIT.class);
-private static final int MAX_LOOKBACK_AGE = 10;
+private static final int MAX_LOOKBACK_AGE = 15;
 private static final int ROWS_POPULATED = 2;
+public static final int WAIT_AFTER_TABLE_CREATION = 60;
 
 Review comment:
   Can we write the unit as part of const name?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Hudson (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015417#comment-17015417
 ] 

Hudson commented on PHOENIX-5644:
-

FAILURE: Integrated in Jenkins build Phoenix-4.x-HBase-1.3 #649 (See 
[https://builds.apache.org/job/Phoenix-4.x-HBase-1.3/649/])
PHOENIX-5644 and PHOENIX-5651 addendum patch (s.kadam: rev 
fac60df981afceacbfbcfb5d04a190fcf349e498)
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapperForTest.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java
* (add) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java


> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.v1.patch
>
>  Time Spent: 6h
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5651) IndexScrutiny does not handle TTL/row-expiry

2020-01-14 Thread Hudson (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015418#comment-17015418
 ] 

Hudson commented on PHOENIX-5651:
-

FAILURE: Integrated in Jenkins build Phoenix-4.x-HBase-1.3 #649 (See 
[https://builds.apache.org/job/Phoenix-4.x-HBase-1.3/649/])
PHOENIX-5644 and PHOENIX-5651 addendum patch (s.kadam: rev 
fac60df981afceacbfbcfb5d04a190fcf349e498)
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapperForTest.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java
* (add) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java


> IndexScrutiny does not handle TTL/row-expiry
> 
>
> Key: PHOENIX-5651
> URL: https://issues.apache.org/jira/browse/PHOENIX-5651
> Project: Phoenix
>  Issue Type: Bug
>Affects Versions: 4.15.1, 4.14.3
>Reporter: Priyank Porwal
>Assignee: Swaroopa Kadam
>Priority: Major
> Fix For: 4.15.1, 4.16.0
>
> Attachments: PHOENIX-5651.4.x-HBase-1.3.patch, 
> PHOENIX-5651.4.x-HBase-1.3.v1.patch, PHOENIX-5651.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5651.master.v1.patch, PHOENIX-5651.master.v2.patch
>
>  Time Spent: 3h 40m
>  Remaining Estimate: 0h
>
> If a data-table has TTL on it, it's indexes inherit the TTL too. Hence when 
> we run IndexScrutiny on such tables and it's indexes, scrutiny's attempts to 
> find matching index rows for near-expiry data rows results in no-matches 
> since the index row gets expired before the read from data-region mapper. The 
> same happens in the MR job for the other direction Index->Data.
> This does not impact correctness of indexing design, but makes it very 
> inconvenient to get a clean scrutiny run. All reported invalid rows have to 
> be matched against the table TTL, which is non-trivial exercise.
> IndexScrutiny itself could detect such expired rows when the matching pair is 
> not found and not report them as INVALID_ROWS. Perhaps a new counter for 
> EXPIRED_ROWS should be added as well for better visibility. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5651) IndexScrutiny does not handle TTL/row-expiry

2020-01-14 Thread Hudson (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015416#comment-17015416
 ] 

Hudson commented on PHOENIX-5651:
-

FAILURE: Integrated in Jenkins build Phoenix-4.x-HBase-1.4 #365 (See 
[https://builds.apache.org/job/Phoenix-4.x-HBase-1.4/365/])
PHOENIX-5644 and PHOENIX-5651 addendum patch (s.kadam: rev 
aa0b3dca19b99d47d4553c6776f31e9cee26c68f)
* (add) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapperForTest.java
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java


> IndexScrutiny does not handle TTL/row-expiry
> 
>
> Key: PHOENIX-5651
> URL: https://issues.apache.org/jira/browse/PHOENIX-5651
> Project: Phoenix
>  Issue Type: Bug
>Affects Versions: 4.15.1, 4.14.3
>Reporter: Priyank Porwal
>Assignee: Swaroopa Kadam
>Priority: Major
> Fix For: 4.15.1, 4.16.0
>
> Attachments: PHOENIX-5651.4.x-HBase-1.3.patch, 
> PHOENIX-5651.4.x-HBase-1.3.v1.patch, PHOENIX-5651.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5651.master.v1.patch, PHOENIX-5651.master.v2.patch
>
>  Time Spent: 3h 40m
>  Remaining Estimate: 0h
>
> If a data-table has TTL on it, it's indexes inherit the TTL too. Hence when 
> we run IndexScrutiny on such tables and it's indexes, scrutiny's attempts to 
> find matching index rows for near-expiry data rows results in no-matches 
> since the index row gets expired before the read from data-region mapper. The 
> same happens in the MR job for the other direction Index->Data.
> This does not impact correctness of indexing design, but makes it very 
> inconvenient to get a clean scrutiny run. All reported invalid rows have to 
> be matched against the table TTL, which is non-trivial exercise.
> IndexScrutiny itself could detect such expired rows when the matching pair is 
> not found and not report them as INVALID_ROWS. Perhaps a new counter for 
> EXPIRED_ROWS should be added as well for better visibility. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Hudson (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015415#comment-17015415
 ] 

Hudson commented on PHOENIX-5644:
-

FAILURE: Integrated in Jenkins build Phoenix-4.x-HBase-1.4 #365 (See 
[https://builds.apache.org/job/Phoenix-4.x-HBase-1.4/365/])
PHOENIX-5644 and PHOENIX-5651 addendum patch (s.kadam: rev 
aa0b3dca19b99d47d4553c6776f31e9cee26c68f)
* (add) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapperForTest.java
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java


> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.v1.patch
>
>  Time Spent: 6h
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Hudson (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015411#comment-17015411
 ] 

Hudson commented on PHOENIX-5644:
-

FAILURE: Integrated in Jenkins build Phoenix-4.x-HBase-1.5 #245 (See 
[https://builds.apache.org/job/Phoenix-4.x-HBase-1.5/245/])
PHOENIX-5644 and PHOENIX-5651 addendum patch (s.kadam: rev 
2aa5616e460fca477e8a213ebc0e5025077230f5)
* (add) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapperForTest.java
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java


> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.v1.patch
>
>  Time Spent: 6h
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5651) IndexScrutiny does not handle TTL/row-expiry

2020-01-14 Thread Hudson (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015412#comment-17015412
 ] 

Hudson commented on PHOENIX-5651:
-

FAILURE: Integrated in Jenkins build Phoenix-4.x-HBase-1.5 #245 (See 
[https://builds.apache.org/job/Phoenix-4.x-HBase-1.5/245/])
PHOENIX-5644 and PHOENIX-5651 addendum patch (s.kadam: rev 
2aa5616e460fca477e8a213ebc0e5025077230f5)
* (add) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapperForTest.java
* (edit) 
phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java
* (edit) 
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java


> IndexScrutiny does not handle TTL/row-expiry
> 
>
> Key: PHOENIX-5651
> URL: https://issues.apache.org/jira/browse/PHOENIX-5651
> Project: Phoenix
>  Issue Type: Bug
>Affects Versions: 4.15.1, 4.14.3
>Reporter: Priyank Porwal
>Assignee: Swaroopa Kadam
>Priority: Major
> Fix For: 4.15.1, 4.16.0
>
> Attachments: PHOENIX-5651.4.x-HBase-1.3.patch, 
> PHOENIX-5651.4.x-HBase-1.3.v1.patch, PHOENIX-5651.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5651.master.v1.patch, PHOENIX-5651.master.v2.patch
>
>  Time Spent: 3h 40m
>  Remaining Estimate: 0h
>
> If a data-table has TTL on it, it's indexes inherit the TTL too. Hence when 
> we run IndexScrutiny on such tables and it's indexes, scrutiny's attempts to 
> find matching index rows for near-expiry data rows results in no-matches 
> since the index row gets expired before the read from data-region mapper. The 
> same happens in the MR job for the other direction Index->Data.
> This does not impact correctness of indexing design, but makes it very 
> inconvenient to get a clean scrutiny run. All reported invalid rows have to 
> be matched against the table TTL, which is non-trivial exercise.
> IndexScrutiny itself could detect such expired rows when the matching pair is 
> not found and not report them as INVALID_ROWS. Perhaps a new counter for 
> EXPIRED_ROWS should be added as well for better visibility. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Geoffrey Jacoby (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015383#comment-17015383
 ] 

Geoffrey Jacoby commented on PHOENIX-5644:
--

[~larsh] - the remaining test failures are mine, and I should have a fix up by 
end of day. 

> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.v1.patch
>
>  Time Spent: 6h
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5672) Unable to find cached index metadata with large UPSERT/SELECT and local index.

2020-01-14 Thread Lars Hofhansl (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015368#comment-17015368
 ] 

Lars Hofhansl commented on PHOENIX-5672:


Btw. I have been disabling this via the setting, and turned off server side 
UPSERT/SELECT, and increased some timeout, and was able to issue 60m row 
upserts on a single server, where previous it would fail around 15m rows.
So again, while it sounds good to do work on the server, it does not scale, and 
it cannot be chunked or paced easily.


> Unable to find cached index metadata with large UPSERT/SELECT and local index.
> --
>
> Key: PHOENIX-5672
> URL: https://issues.apache.org/jira/browse/PHOENIX-5672
> Project: Phoenix
>  Issue Type: Bug
>Affects Versions: 4.15.0
>Reporter: Lars Hofhansl
>Priority: Major
>
> Doing a very large UPSERT/SELECT back into the same table. After a while I 
> get this exception. This happens with server side mutation turned off or on 
> and regardless of the batch-size (which I have increased to 1 in this 
> last example).
> {code:java}
> 20/01/10 16:41:54 WARN client.AsyncProcess: #1, table=TEST, attempt=1/35 
> failed=1ops, last exception: 
> org.apache.hadoop.hbase.DoNotRetryIOException: 
> org.apache.hadoop.hbase.DoNotRetryIOException: ERROR 2008 (INT10): ERROR 2008 
> (INT10): Unable to find cached index metadata.  key=-1180967500149768360 
> region=TEST,\x80\x965g\x80\x0F@\xAA\x80Y$\xEF,1578504217187.42467236e0b49fda05fdaaf69de98832.host=lhofhansl-wsl2,16201,157870268
>  Index update failed20/01/10 16:41:54 WARN client.AsyncProcess: #1, 
> table=TEST, attempt=1/35 failed=1ops, last exception: 
> org.apache.hadoop.hbase.DoNotRetryIOException: 
> org.apache.hadoop.hbase.DoNotRetryIOException: ERROR 2008 (INT10): ERROR 2008 
> (INT10): Unable to find cached index metadata.  key=-1180967500149768360 
> region=TEST,\x80\x965g\x80\x0F@\xAA\x80Y$\xEF,1578504217187.42467236e0b49fda05fdaaf69de98832.host=lhofhansl-wsl2,16201,157870268
>  Index update failed at 
> org.apache.phoenix.util.ServerUtil.createIOException(ServerUtil.java:113) at 
> org.apache.phoenix.util.ServerUtil.throwIOException(ServerUtil.java:87) at 
> org.apache.phoenix.index.PhoenixIndexMetaDataBuilder.getIndexMetaDataCache(PhoenixIndexMetaDataBuilder.java:101)
>  at 
> org.apache.phoenix.index.PhoenixIndexMetaDataBuilder.getIndexMetaData(PhoenixIndexMetaDataBuilder.java:51)
>  at 
> org.apache.phoenix.index.PhoenixIndexBuilder.getIndexMetaData(PhoenixIndexBuilder.java:100)
>  at 
> org.apache.phoenix.index.PhoenixIndexBuilder.getIndexMetaData(PhoenixIndexBuilder.java:73)
>  at 
> org.apache.phoenix.hbase.index.builder.IndexBuildManager.getIndexMetaData(IndexBuildManager.java:84)
>  at 
> org.apache.phoenix.hbase.index.IndexRegionObserver.getPhoenixIndexMetaData(IndexRegionObserver.java:594)
>  at 
> org.apache.phoenix.hbase.index.IndexRegionObserver.preBatchMutateWithExceptions(IndexRegionObserver.java:646)
>  at 
> org.apache.phoenix.hbase.index.IndexRegionObserver.preBatchMutate(IndexRegionObserver.java:334)
>  at 
> org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost$35.call(RegionCoprocessorHost.java:1024)
>  at 
> org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost$RegionOperation.call(RegionCoprocessorHost.java:1742)
>  at 
> org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost.execOperation(RegionCoprocessorHost.java:1827)
>  at 
> org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost.execOperation(RegionCoprocessorHost.java:1783)
>  at 
> org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost.preBatchMutate(RegionCoprocessorHost.java:1020)
>  at 
> org.apache.hadoop.hbase.regionserver.HRegion.doMiniBatchMutation(HRegion.java:3425)
>  at 
> org.apache.hadoop.hbase.regionserver.HRegion.batchMutate(HRegion.java:3163) 
> at 
> org.apache.hadoop.hbase.regionserver.HRegion.batchMutate(HRegion.java:3105) 
> at 
> org.apache.hadoop.hbase.regionserver.RSRpcServices.doBatchOp(RSRpcServices.java:944)
>  at 
> org.apache.hadoop.hbase.regionserver.RSRpcServices.doNonAtomicRegionMutation(RSRpcServices.java:872)
>  at 
> org.apache.hadoop.hbase.regionserver.RSRpcServices.multi(RSRpcServices.java:2472)
>  at 
> org.apache.hadoop.hbase.protobuf.generated.ClientProtos$ClientService$2.callBlockingMethod(ClientProtos.java:36812)
>  at org.apache.hadoop.hbase.ipc.RpcServer.call(RpcServer.java:2399) at 
> org.apache.hadoop.hbase.ipc.CallRunner.run(CallRunner.java:124) at 
> org.apache.hadoop.hbase.ipc.RpcExecutor$Handler.run(RpcExecutor.java:311) at 
> org.apache.hadoop.hbase.ipc.RpcExecutor$Handler.run(RpcExecutor.java:291)Caused
>  by: java.sql.SQLException: ERROR 2008 (INT10): Unable to find cached index 
> metadata.  key=-1180967500149768360 
> 

[jira] [Commented] (PHOENIX-5629) Phoenix Function to Return HBase Timestamp of Column Cell

2020-01-14 Thread Priyank Porwal (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5629?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015367#comment-17015367
 ] 

Priyank Porwal commented on PHOENIX-5629:
-

[~sakshamgangwar], [~tkhurana] has already started work on this Jira.

> Phoenix Function to Return HBase Timestamp of Column Cell
> -
>
> Key: PHOENIX-5629
> URL: https://issues.apache.org/jira/browse/PHOENIX-5629
> Project: Phoenix
>  Issue Type: New Feature
>Reporter: Geoffrey Jacoby
>Assignee: Tanuj Khurana
>Priority: Major
> Fix For: 5.1.0, 4.16.0
>
>
> t's occasionally useful when diagnosing an issue with Phoenix to be able to 
> easily look up the HBase timestamp of the HBase Cell returned by a Phoenix 
> query. 
> For example:
> SELECT ROW_TIMESTAMP(Column1) FROM Table1 WHERE Column1 = 'SomeValue'



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5651) IndexScrutiny does not handle TTL/row-expiry

2020-01-14 Thread Hadoop QA (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015354#comment-17015354
 ] 

Hadoop QA commented on PHOENIX-5651:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  
http://issues.apache.org/jira/secure/attachment/12990908/PHOENIX-5651.master.v1.patch
  against master branch at commit 6405839b60c410292c0e2c8cbbdee55d09343bab.
  ATTACHMENT ID: 12990908

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:red}-1 tests included{color}.  The patch doesn't appear to include 
any new or modified tests.
Please justify why no new tests are needed for this 
patch.
Also please list what manual steps were performed to 
verify this patch.

{color:red}-1 javac{color}.  The patch appears to cause mvn compile goal to 
fail .

Compilation errors resume:
[ERROR] COMPILATION ERROR : 
[ERROR] 
/home/jenkins/jenkins-slave/workspace/PreCommit-PHOENIX-Build/phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexScrutinyToolIT.java:[426,33]
 cannot find symbol
[ERROR] Failed to execute goal 
org.apache.maven.plugins:maven-compiler-plugin:3.7.0:testCompile 
(default-testCompile) on project phoenix-core: Compilation failure
[ERROR] 
/home/jenkins/jenkins-slave/workspace/PreCommit-PHOENIX-Build/phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexScrutinyToolIT.java:[426,33]
 cannot find symbol
[ERROR]   symbol:   variable Sets
[ERROR]   location: class org.apache.phoenix.end2end.IndexScrutinyToolIT
[ERROR] 
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e 
switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please 
read the following articles:
[ERROR] [Help 1] 
http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn  -rf :phoenix-core


Console output: 
https://builds.apache.org/job/PreCommit-PHOENIX-Build/3291//console

This message is automatically generated.

> IndexScrutiny does not handle TTL/row-expiry
> 
>
> Key: PHOENIX-5651
> URL: https://issues.apache.org/jira/browse/PHOENIX-5651
> Project: Phoenix
>  Issue Type: Bug
>Affects Versions: 4.15.1, 4.14.3
>Reporter: Priyank Porwal
>Assignee: Swaroopa Kadam
>Priority: Major
> Fix For: 4.15.1, 4.16.0
>
> Attachments: PHOENIX-5651.4.x-HBase-1.3.patch, 
> PHOENIX-5651.4.x-HBase-1.3.v1.patch, PHOENIX-5651.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5651.master.v1.patch
>
>  Time Spent: 3h 40m
>  Remaining Estimate: 0h
>
> If a data-table has TTL on it, it's indexes inherit the TTL too. Hence when 
> we run IndexScrutiny on such tables and it's indexes, scrutiny's attempts to 
> find matching index rows for near-expiry data rows results in no-matches 
> since the index row gets expired before the read from data-region mapper. The 
> same happens in the MR job for the other direction Index->Data.
> This does not impact correctness of indexing design, but makes it very 
> inconvenient to get a clean scrutiny run. All reported invalid rows have to 
> be matched against the table TTL, which is non-trivial exercise.
> IndexScrutiny itself could detect such expired rows when the matching pair is 
> not found and not report them as INVALID_ROWS. Perhaps a new counter for 
> EXPIRED_ROWS should be added as well for better visibility. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Swaroopa Kadam (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015343#comment-17015343
 ] 

Swaroopa Kadam commented on PHOENIX-5644:
-

Thank you, Lars!  Appreciate your help and patience. 

> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.v1.patch
>
>  Time Spent: 6h
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] swaroopak opened a new pull request #681: PHOENIX-5651_master: IndexScrutiny does not handle TTL/row-expiry

2020-01-14 Thread GitBox
swaroopak opened a new pull request #681: PHOENIX-5651_master: IndexScrutiny 
does not handle TTL/row-expiry
URL: https://github.com/apache/phoenix/pull/681
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Comment Edited] (PHOENIX-5644) IndexUpgradeTool should sleep only once if there is at least one immutable table provided

2020-01-14 Thread Lars Hofhansl (Jira)


[ 
https://issues.apache.org/jira/browse/PHOENIX-5644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17015331#comment-17015331
 ] 

Lars Hofhansl edited comment on PHOENIX-5644 at 1/14/20 7:20 PM:
-

Hmm... I would agree. (Although we should not have any flappers anymore - it 
might have been a followup failure, which is exactly why it's not good to have 
tests broken).
If you're sure that this fixes the timeout issue, go ahead to commit... +1



was (Author: lhofhansl):
Hmm... I would agree. (Although we should not have any flappers anymore).
If you're sure that this fixes the timeout issue, go ahead to commit... +1

> IndexUpgradeTool should sleep only once if there is at least one immutable 
> table provided
> -
>
> Key: PHOENIX-5644
> URL: https://issues.apache.org/jira/browse/PHOENIX-5644
> Project: Phoenix
>  Issue Type: Improvement
>Affects Versions: 4.14.3
>Reporter: Swaroopa Kadam
>Assignee: Swaroopa Kadam
>Priority: Minor
> Fix For: 5.1.0, 4.15.1, 4.14.4, 4.16.0
>
> Attachments: PHOENIX-5644.4.x-HBase-1.3.add.patch, 
> PHOENIX-5644.4.x-HBase-1.3.add.v1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv1.patch, 
> PHOENIX-5644.4.x-HBase-1.3.addv2.patch, PHOENIX-5644.4.x-HBase-1.3.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v1.patch, PHOENIX-5644.4.x-HBase-1.3.v2.patch, 
> PHOENIX-5644.4.x-HBase-1.3.v3.patch, PHOENIX-5644.v1.patch
>
>  Time Spent: 6h
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [phoenix] ntshmah edited a comment on issue #659: PHOENIX-5634: Use 'phoenix.default.update.cache.frequency' from conne…

2020-01-14 Thread GitBox
ntshmah edited a comment on issue #659: PHOENIX-5634: Use 
'phoenix.default.update.cache.frequency' from conne…
URL: https://github.com/apache/phoenix/pull/659#issuecomment-574323990
 
 
   Hi @ChinmaySKulkarni 
   I will be squashing my commits together and force pushing here since quite a 
few things changed from the time this was last reviewed. I hope that is fine.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [phoenix] ntshmah commented on issue #659: PHOENIX-5634: Use 'phoenix.default.update.cache.frequency' from conne…

2020-01-14 Thread GitBox
ntshmah commented on issue #659: PHOENIX-5634: Use 
'phoenix.default.update.cache.frequency' from conne…
URL: https://github.com/apache/phoenix/pull/659#issuecomment-574323990
 
 
   Hi @ckulkarni
   I will be squashing my commits together and force pushing here since quite a 
few things changed from the time this was last reviewed. I hope that is fine.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


  1   2   >