[jira] [Commented] (PHOENIX-3161) Check possibility of moving rebuilding code to coprocessor of data table.
[ https://issues.apache.org/jira/browse/PHOENIX-3161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15595307#comment-15595307 ] Hudson commented on PHOENIX-3161: - SUCCESS: Integrated in Jenkins build Phoenix-master #1446 (See [https://builds.apache.org/job/Phoenix-master/1446/]) PHOENIX-3161 Check possibility of moving rebuilding code to coprocessor (ankitsinghal59: rev a95e8ab1af2b8defef3d8c0ed5a060c9b9881dd9) * (edit) phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexFailureIT.java * (edit) phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java * (edit) phoenix-core/src/main/java/org/apache/phoenix/compile/PostDDLCompiler.java * (edit) phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java * (edit) phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java * (edit) phoenix-core/src/main/java/org/apache/phoenix/hbase/index/util/IndexManagementUtil.java * (edit) phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataRegionObserver.java > Check possibility of moving rebuilding code to coprocessor of data table. > - > > Key: PHOENIX-3161 > URL: https://issues.apache.org/jira/browse/PHOENIX-3161 > Project: Phoenix > Issue Type: Sub-task >Reporter: Ankit Singhal >Assignee: Ankit Singhal > Fix For: 4.9.0 > > Attachments: PHOENIX-3161.patch, PHOENIX-3161_v1.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-3161) Check possibility of moving rebuilding code to coprocessor of data table.
[ https://issues.apache.org/jira/browse/PHOENIX-3161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15595219#comment-15595219 ] Ankit Singhal commented on PHOENIX-3161: Thanks [~jamestaylor] for the review. committed to 4.x branch and master. > Check possibility of moving rebuilding code to coprocessor of data table. > - > > Key: PHOENIX-3161 > URL: https://issues.apache.org/jira/browse/PHOENIX-3161 > Project: Phoenix > Issue Type: Sub-task >Reporter: Ankit Singhal >Assignee: Ankit Singhal > Fix For: 4.9.0 > > Attachments: PHOENIX-3161.patch, PHOENIX-3161_v1.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-3161) Check possibility of moving rebuilding code to coprocessor of data table.
[ https://issues.apache.org/jira/browse/PHOENIX-3161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15592073#comment-15592073 ] James Taylor commented on PHOENIX-3161: --- Yes, you're right, [~an...@apache.org]. +1 to the patch. > Check possibility of moving rebuilding code to coprocessor of data table. > - > > Key: PHOENIX-3161 > URL: https://issues.apache.org/jira/browse/PHOENIX-3161 > Project: Phoenix > Issue Type: Sub-task >Reporter: Ankit Singhal >Assignee: Ankit Singhal > Fix For: 4.9.0 > > Attachments: PHOENIX-3161.patch, PHOENIX-3161_v1.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-3161) Check possibility of moving rebuilding code to coprocessor of data table.
[
https://issues.apache.org/jira/browse/PHOENIX-3161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15591590#comment-15591590
]
Ankit Singhal commented on PHOENIX-3161:
Below code should give smallest value of minTimeStamp only right?
{code}
long minTimeStamp = 0;
// get the minimum timestamp across all the mutations we attempted
on that table
Collection mutations = attempted.get(ref);
if (mutations != null) {
for (Mutation m : mutations) {
for (List kvs : m.getFamilyCellMap().values()) {
for (Cell kv : kvs) {
if (minTimeStamp == 0 || (kv.getTimestamp() >= 0 &&
minTimeStamp > kv.getTimestamp())) {
minTimeStamp = kv.getTimestamp();
}
}
}
}
}
{code}
> Check possibility of moving rebuilding code to coprocessor of data table.
> -
>
> Key: PHOENIX-3161
> URL: https://issues.apache.org/jira/browse/PHOENIX-3161
> Project: Phoenix
> Issue Type: Sub-task
>Reporter: Ankit Singhal
>Assignee: Ankit Singhal
> Fix For: 4.9.0
>
> Attachments: PHOENIX-3161.patch, PHOENIX-3161_v1.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-3161) Check possibility of moving rebuilding code to coprocessor of data table.
[ https://issues.apache.org/jira/browse/PHOENIX-3161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15587995#comment-15587995 ] James Taylor commented on PHOENIX-3161: --- bq. We have already fix it in other JIRA(PHOENIX-3331), the change was to get minTimeStamp of all KVs only. I don't think this fix is correct - we should change it back. We want minTimeStamp to be the smallest value of all the kv.getTimestamp() we see. It's basically "how far back in time do we need to query". What do you think? > Check possibility of moving rebuilding code to coprocessor of data table. > - > > Key: PHOENIX-3161 > URL: https://issues.apache.org/jira/browse/PHOENIX-3161 > Project: Phoenix > Issue Type: Sub-task >Reporter: Ankit Singhal >Assignee: Ankit Singhal > Fix For: 4.9.0 > > Attachments: PHOENIX-3161.patch, PHOENIX-3161_v1.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-3161) Check possibility of moving rebuilding code to coprocessor of data table.
[
https://issues.apache.org/jira/browse/PHOENIX-3161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15585181#comment-15585181
]
Hadoop QA commented on PHOENIX-3161:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12833919/PHOENIX-3161_v1.patch
against master branch at commit 1e78d3b368b7aa8397224074f691ea2fed340e34.
ATTACHMENT ID: 12833919
{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:red}-1 javadoc{color}. The javadoc tool appears to have generated
43 warning messages.
{color:red}-1 release audit{color}. The applied patch generated 1 release
audit warnings (more than the master's current 0 warnings).
{color:red}-1 lineLengths{color}. The patch introduces the following lines
longer than 100:
++
SchemaUtil.getPhysicalTableName(fullTableName.getBytes(),
isNamespaceMapped)+"\nCLIENT MERGE SORT";
++
SchemaUtil.getPhysicalTableName(fullTableName.getBytes(),
isNamespaceMapped)+"\nCLIENT MERGE SORT";
+TableRef tableRef = new TableRef(null, dataPTable,
HConstants.LATEST_TIMESTAMP, false);
+MutationPlan plan =
compiler.compile(Collections.singletonList(tableRef), null, null, null,
+Scan dataTableScan =
IndexManagementUtil.newLocalStateScan(plan.getContext().getScan(),
+
dataTableScan.setAttribute(BaseScannerRegionObserver.REBUILD_INDEXES,
TRUE_BYTES);
+IndexMaintainer.serializeAdditional(dataPTable,
indexMetaDataPtr, indexesToPartiallyRebuild,
+} else if (ScanUtil.isIndexRebuild(scan)) { return rebuildIndices(s,
region, scan, env.getConfiguration()); }
+private RegionScanner rebuildIndices(final RegionScanner innerScanner,
final Region region, final Scan scan,
+int batchSize = config.getInt(MUTATE_BATCH_SIZE_ATTRIB,
QueryServicesOptions.DEFAULT_MUTATE_BATCH_SIZE);
{color:green}+1 core tests{color}. The patch passed unit tests in .
Test results:
https://builds.apache.org/job/PreCommit-PHOENIX-Build/625//testReport/
Release audit warnings:
https://builds.apache.org/job/PreCommit-PHOENIX-Build/625//artifact/patchprocess/patchReleaseAuditWarnings.txt
Javadoc warnings:
https://builds.apache.org/job/PreCommit-PHOENIX-Build/625//artifact/patchprocess/patchJavadocWarnings.txt
Console output:
https://builds.apache.org/job/PreCommit-PHOENIX-Build/625//console
This message is automatically generated.
> Check possibility of moving rebuilding code to coprocessor of data table.
> -
>
> Key: PHOENIX-3161
> URL: https://issues.apache.org/jira/browse/PHOENIX-3161
> Project: Phoenix
> Issue Type: Sub-task
>Reporter: Ankit Singhal
>Assignee: Ankit Singhal
> Fix For: 4.9.0
>
> Attachments: PHOENIX-3161.patch, PHOENIX-3161_v1.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-3161) Check possibility of moving rebuilding code to coprocessor of data table.
[
https://issues.apache.org/jira/browse/PHOENIX-3161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15585071#comment-15585071
]
Ankit Singhal commented on PHOENIX-3161:
bq. Does the HBase copy constructor not maintain the rawScan and cacheBlocks
values correctly? If not can you comment there and reference the HBase bug for
those?
Yes, Scan copy constructor is copying attributes and cachBlocks property. so
removed the extra updates to newScan from patch.
bq. Don't change the setTimeRange(Scan,long) method, but use the
setTimeRange(Scan,long,long) method instead. I'm not sure all the callers would
expect the min time range to come from the Scan passed in. Will that work for
you?
Yes, updated the patch to use setTimeRange(Scan,long,long) ,though
setTimeRange(Scan,long) was getting used by PostDDLCompiler.
bq. I don't think this fix is correct (and maybe you already made a related
fixed that we should look at too, though perhaps I reworked that already?). We
want minTimeStamp to be the smallest value of all the kv.getTimestamp() we see.
It's basically "how far back in time do we need to query".
We have already fix it in other JIRA(PHOENIX-3331), the change was to get
minTimeStamp of all KVs only.
{code}
- if (minTimeStamp == 0 || (kv.getTimestamp() >= 0 && minTimeStamp <
kv.getTimestamp())) {
+if (minTimeStamp == 0 || (kv.getTimestamp() >= 0
&& minTimeStamp > kv.getTimestamp())) {
{code}
bq. I thought that region.batchMutate() won't invoke coprocessors like
htable.batch() does?
Yes ,it call coprocessors like htable.batch().
bq. Do existing tests pass with your patch?
yes all tests were passed. MutableIndexIT is mainly testing this.
bq. FWIW, we'd like to change the coprocessor code to use htable.batch()
instead (see PHOENIX-3271) so that we can execute UPSERT SELECT on the
server-side instead of bringing all the data back to the client-side, even if
the source and target tables are different (if auto commit is on). Would love
it if you could tackle that one next.
sure, I can try to take it next.
> Check possibility of moving rebuilding code to coprocessor of data table.
> -
>
> Key: PHOENIX-3161
> URL: https://issues.apache.org/jira/browse/PHOENIX-3161
> Project: Phoenix
> Issue Type: Sub-task
>Reporter: Ankit Singhal
>Assignee: Ankit Singhal
> Fix For: 4.9.0
>
> Attachments: PHOENIX-3161.patch, PHOENIX-3161_v1.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-3161) Check possibility of moving rebuilding code to coprocessor of data table.
[
https://issues.apache.org/jira/browse/PHOENIX-3161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15583219#comment-15583219
]
James Taylor commented on PHOENIX-3161:
---
I like this approach overall, [~an...@apache.org]. Nice work! Here's some
specific feedback:
- Does the HBase copy constructor not maintain the rawScan and cacheBlocks
values correctly? If not can you comment there and reference the HBase bug for
those?
- Don't change the setTimeRange(Scan,long) method, but use the
setTimeRange(Scan,long,long) method instead. I'm not sure all the callers would
expect the min time range to come from the Scan passed in. Will that work for
you?
{code}
@@ -142,6 +141,8 @@ public class ScanUtil {
TreeMap> existingMap =
(TreeMap>)scan.getFamilyMap();
Map> clonedMap = new TreeMap>(existingMap);
newScan.setFamilyMap(clonedMap);
+newScan.setRaw(scan.isRaw());
+newScan.setCacheBlocks(scan.getCacheBlocks());
// Carry over the reversed attribute
newScan.setReversed(scan.isReversed());
newScan.setSmall(scan.isSmall());
@@ -269,7 +270,7 @@ public class ScanUtil {
public static void setTimeRange(Scan scan, long ts) {
try {
-scan.setTimeRange(MetaDataProtocol.MIN_TABLE_TIMESTAMP, ts);
+scan.setTimeRange(scan.getTimeRange().getMin(), ts);
} catch (IOException e) {
throw new RuntimeException(e);
}
{code}
- I don't think this fix is correct (and maybe you already made a related fixed
that we should look at too, though perhaps I reworked that already?). We want
minTimeStamp to be the smallest value of all the kv.getTimestamp() we see. It's
basically "how far back in time do we need to query".
{code}
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexFailurePolicy.java
b/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexFailurePolicy.java
index c29d7b4..eb73d6b 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexFailurePolicy.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexFailurePolicy.java
@@ -129,7 +129,7 @@ public class PhoenixIndexFailurePolicy extends
DelegateIndexFailurePolicy {
for (Mutation m : mutations) {
for (List kvs : m.getFamilyCellMap().values()) {
for (Cell kv : kvs) {
-if (minTimeStamp == 0 || (kv.getTimestamp() >= 0
&& minTimeStamp < kv.getTimestamp())) {
+if (minTimeStamp == 0 || (kv.getTimestamp() >= 0
&& minTimeStamp > kv.getTimestamp())) {
minTimeStamp = kv.getTimestamp();
}
}
{code}
- I thought that region.batchMutate() won't invoke coprocessors like
htable.batch() does? Do existing tests pass with your patch? FWIW, we'd like to
change the coprocessor code to use htable.batch() instead (see PHOENIX-3271) so
that we can execute UPSERT SELECT on the server-side instead of bringing all
the data back to the client-side, even if the source and target tables are
different (if auto commit is on). Would love it if you could tackle that one
next.
> Check possibility of moving rebuilding code to coprocessor of data table.
> -
>
> Key: PHOENIX-3161
> URL: https://issues.apache.org/jira/browse/PHOENIX-3161
> Project: Phoenix
> Issue Type: Sub-task
>Reporter: Ankit Singhal
>Assignee: Ankit Singhal
> Fix For: 4.9.0
>
> Attachments: PHOENIX-3161.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-3161) Check possibility of moving rebuilding code to coprocessor of data table.
[ https://issues.apache.org/jira/browse/PHOENIX-3161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15581718#comment-15581718 ] Ankit Singhal commented on PHOENIX-3161: ping [~jamestaylor], can you please review the attached patch. > Check possibility of moving rebuilding code to coprocessor of data table. > - > > Key: PHOENIX-3161 > URL: https://issues.apache.org/jira/browse/PHOENIX-3161 > Project: Phoenix > Issue Type: Sub-task >Reporter: Ankit Singhal >Assignee: Ankit Singhal > Fix For: 4.9.0 > > Attachments: PHOENIX-3161.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-3161) Check possibility of moving rebuilding code to coprocessor of data table.
[
https://issues.apache.org/jira/browse/PHOENIX-3161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15529925#comment-15529925
]
Hadoop QA commented on PHOENIX-3161:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12830693/PHOENIX-3161.patch
against master branch at commit 58596bbc416ce577d3407910f1a127150d8c.
ATTACHMENT ID: 12830693
{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:red}-1 javadoc{color}. The javadoc tool appears to have generated
38 warning messages.
{color:red}-1 release audit{color}. The applied patch generated 1 release
audit warnings (more than the master's current 0 warnings).
{color:red}-1 lineLengths{color}. The patch introduces the following lines
longer than 100:
++
SchemaUtil.getPhysicalTableName(fullTableName.getBytes(),
isNamespaceMapped)+"\nCLIENT MERGE SORT";
++
SchemaUtil.getPhysicalTableName(fullTableName.getBytes(),
isNamespaceMapped)+"\nCLIENT MERGE SORT";
+TableRef tableRef = new TableRef(null, dataPTable,
HConstants.LATEST_TIMESTAMP, false);
+MutationPlan plan =
compiler.compile(Collections.singletonList(tableRef), null, null, null,
+Scan dataTableScan =
IndexManagementUtil.newLocalStateScan(plan.getContext().getScan(),
+
dataTableScan.setAttribute(BaseScannerRegionObserver.REBUILD_INDEXES,
TRUE_BYTES);
+IndexMaintainer.serializeAdditional(dataPTable,
indexMetaDataPtr, indexesToPartiallyRebuild,
+} else if (ScanUtil.isIndexRebuild(scan)) { return rebuildIndices(s,
region, scan, env.getConfiguration()); }
+private RegionScanner rebuildIndices(final RegionScanner innerScanner,
final Region region, final Scan scan,
+int batchSize = config.getInt(MUTATE_BATCH_SIZE_ATTRIB,
QueryServicesOptions.DEFAULT_MUTATE_BATCH_SIZE);
{color:green}+1 core tests{color}. The patch passed unit tests in .
{color:red}-1 core zombie tests{color}. There are 1 zombie test(s):
at org.apache.oozie.test.MiniHCatServer$1.run(MiniHCatServer.java:137)
at
org.apache.oozie.test.XTestCase$MiniClusterShutdownMonitor.run(XTestCase.java:1114)
at org.apache.oozie.test.XTestCase.waitFor(XTestCase.java:713)
at
org.apache.oozie.action.hadoop.TestHiveActionExecutor.testHiveAction(TestHiveActionExecutor.java:168)
Test results:
https://builds.apache.org/job/PreCommit-PHOENIX-Build/601//testReport/
Release audit warnings:
https://builds.apache.org/job/PreCommit-PHOENIX-Build/601//artifact/patchprocess/patchReleaseAuditWarnings.txt
Javadoc warnings:
https://builds.apache.org/job/PreCommit-PHOENIX-Build/601//artifact/patchprocess/patchJavadocWarnings.txt
Console output:
https://builds.apache.org/job/PreCommit-PHOENIX-Build/601//console
This message is automatically generated.
> Check possibility of moving rebuilding code to coprocessor of data table.
> -
>
> Key: PHOENIX-3161
> URL: https://issues.apache.org/jira/browse/PHOENIX-3161
> Project: Phoenix
> Issue Type: Sub-task
>Reporter: Ankit Singhal
>Assignee: Ankit Singhal
> Fix For: 4.9.0
>
> Attachments: PHOENIX-3161.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
