You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2022/08/24 00:49:57 UTC
[GitHub] [phoenix] chrajeshbabu opened a new pull request, #1493: PHOENIX-6767 Traversing through all the guideposts to prepare paralle…
chrajeshbabu opened a new pull request, #1493:
URL: https://github.com/apache/phoenix/pull/1493
…l scans is not required for salted tables when the query is point lookup
--
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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] chrajeshbabu commented on pull request #1493: PHOENIX-6767 Traversing through all the guideposts to prepare paralle…
Posted by GitBox <gi...@apache.org>.
chrajeshbabu commented on PR #1493:
URL: https://github.com/apache/phoenix/pull/1493#issuecomment-1247076526
> @chrajeshbabu - looks like there are test failures in PhoenixTableLevelMetricsIT -- do the tests need to be updated with different metric values to assert on now that the query planning is more efficient in some cases?
Since we are not calling get table regions API here custom connection queries don't have effect and ignored the tests.
@gjacoby126
--
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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] chrajeshbabu commented on pull request #1493: PHOENIX-6767 Traversing through all the guideposts to prepare paralle…
Posted by GitBox <gi...@apache.org>.
chrajeshbabu commented on PR #1493:
URL: https://github.com/apache/phoenix/pull/1493#issuecomment-1247076884
Thanks for reviews @stoty @gjacoby126 .
--
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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] gjacoby126 commented on pull request #1493: PHOENIX-6767 Traversing through all the guideposts to prepare paralle…
Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on PR #1493:
URL: https://github.com/apache/phoenix/pull/1493#issuecomment-1246064309
@chrajeshbabu - looks like there are test failures in PhoenixTableLevelMetricsIT -- do the tests need to be updated with different metric values to assert on now that the query planning is more efficient in some cases?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] chrajeshbabu commented on a diff in pull request #1493: PHOENIX-6767 Traversing through all the guideposts to prepare paralle…
Posted by GitBox <gi...@apache.org>.
chrajeshbabu commented on code in PR #1493:
URL: https://github.com/apache/phoenix/pull/1493#discussion_r971096883
##########
phoenix-core/src/main/java/org/apache/phoenix/iterate/BaseResultIterators.java:
##########
@@ -929,12 +929,19 @@ private static boolean clipKeyRangeBytes(RowKeySchema schema, int fieldIndex, in
* @throws SQLException
*/
private List<List<Scan>> getParallelScans(byte[] startKey, byte[] stopKey) throws SQLException {
- List<HRegionLocation> regionLocations = getRegionBoundaries(scanGrouper);
- List<byte[]> regionBoundaries = toBoundaries(regionLocations);
ScanRanges scanRanges = context.getScanRanges();
PTable table = getTable();
- boolean isSalted = table.getBucketNum() != null;
boolean isLocalIndex = table.getIndexType() == IndexType.LOCAL;
+ if(!isLocalIndex && scanRanges.isPointLookup()) {
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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on a diff in pull request #1493: PHOENIX-6767 Traversing through all the guideposts to prepare paralle…
Posted by GitBox <gi...@apache.org>.
stoty commented on code in PR #1493:
URL: https://github.com/apache/phoenix/pull/1493#discussion_r970446421
##########
phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java:
##########
@@ -549,6 +550,7 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa
assertSelectQueryTableMetrics(tableName, false, 1, 0, 1, 0, 0, true, 0, 0, rs);
}
+ @Ignore
Review Comment:
nit: Can you add coments here ?
##########
phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java:
##########
@@ -549,6 +550,7 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa
assertSelectQueryTableMetrics(tableName, false, 1, 0, 1, 0, 0, true, 0, 0, rs);
}
+ @Ignore
Review Comment:
nit: Can you add comments 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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] chrajeshbabu merged pull request #1493: PHOENIX-6767 Traversing through all the guideposts to prepare paralle…
Posted by GitBox <gi...@apache.org>.
chrajeshbabu merged PR #1493:
URL: https://github.com/apache/phoenix/pull/1493
--
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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on a diff in pull request #1493: PHOENIX-6767 Traversing through all the guideposts to prepare paralle…
Posted by GitBox <gi...@apache.org>.
stoty commented on code in PR #1493:
URL: https://github.com/apache/phoenix/pull/1493#discussion_r955727536
##########
phoenix-core/src/main/java/org/apache/phoenix/iterate/BaseResultIterators.java:
##########
@@ -929,12 +929,19 @@ private static boolean clipKeyRangeBytes(RowKeySchema schema, int fieldIndex, in
* @throws SQLException
*/
private List<List<Scan>> getParallelScans(byte[] startKey, byte[] stopKey) throws SQLException {
- List<HRegionLocation> regionLocations = getRegionBoundaries(scanGrouper);
- List<byte[]> regionBoundaries = toBoundaries(regionLocations);
ScanRanges scanRanges = context.getScanRanges();
PTable table = getTable();
- boolean isSalted = table.getBucketNum() != null;
boolean isLocalIndex = table.getIndexType() == IndexType.LOCAL;
+ if(!isLocalIndex && scanRanges.isPointLookup()) {
Review Comment:
nit:This is rightfully flagged by checkstyle
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/salted/SaltedTableIT.java:
##########
@@ -62,6 +62,33 @@ public void testTableWithInvalidBucketNumber() throws Exception {
}
}
+ @Test
+ public void testPointLookupOnSaltedTable() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ try (Connection conn = DriverManager.getConnection(getUrl(), props);) {
+ String tableName = generateUniqueName();
+ String query = "create table " + tableName + " (a_integer integer not null "
+ + "CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS = 256";
Review Comment:
Doesn't creating this many regions slow down the test ?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] chrajeshbabu commented on a diff in pull request #1493: PHOENIX-6767 Traversing through all the guideposts to prepare paralle…
Posted by GitBox <gi...@apache.org>.
chrajeshbabu commented on code in PR #1493:
URL: https://github.com/apache/phoenix/pull/1493#discussion_r971097655
##########
phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java:
##########
@@ -549,6 +550,7 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa
assertSelectQueryTableMetrics(tableName, false, 1, 0, 1, 0, 0, true, 0, 0, rs);
}
+ @Ignore
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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org