You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "virajjasani (via GitHub)" <gi...@apache.org> on 2024/04/12 23:50:44 UTC
[PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
virajjasani opened a new pull request, #1880:
URL: https://github.com/apache/phoenix/pull/1880
Jira: PHOENIX-7306
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1565844224
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -780,6 +775,10 @@ public List<HRegionLocation> getTableRegions(byte[] tableName, byte[] startRowKe
config.getInt(PHOENIX_GET_REGIONS_RETRIES, DEFAULT_PHOENIX_GET_REGIONS_RETRIES);
TableName table = TableName.valueOf(tableName);
byte[] currentKey = null;
+ final int queryTimeout = this.getProps().getInt(QueryServices.THREAD_TIMEOUT_MS_ATTRIB,
Review Comment:
If it is not overridden at the statement level the statementcontext API gets it from cqsi
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1575137874
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java:
##########
@@ -99,22 +99,48 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated
public TableDescriptor getTableDescriptor(byte[] tableName) throws SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[] row) throws SQLException;
+
+ /**
+ * Retrieve the region metadata locations for all regions of the given table.
+ * This method is Deprecated. Use {@link #getAllTableRegions(byte[], int)} instead.
+ *
+ * @param tableName The table name.
+ * @return The list of table region locations.
+ * @throws SQLException If fails to retrieve region locations.
+ */
+ @Deprecated
public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws SQLException;
+ /**
+ * Retrieve the region metadata locations for all regions of the given table.
+ * The operation to retrieve the table region locations must be completed within
+ * the query timeout.
+ *
+ * @param tableName Table name.
+ * @param queryTimeout Phoenix query timeout.
+ * @return The list of region locations.
+ * @throws SQLException If fails to retrieve region locations.
+ */
+ public List<HRegionLocation> getAllTableRegions(byte[] tableName, int queryTimeout)
+ throws SQLException;
+
/**
* Retrieve table region locations that cover the startRowKey and endRowKey. The start key
* of the first region of the returned list must be less than or equal to startRowKey.
* The end key of the last region of the returned list must be greater than or equal to
- * endRowKey.
+ * endRowKey. The operation to retrieve the table region locations must be completed within
+ * the query timeout.
*
- * @param tableName Table name.
- * @param startRowKey Start RowKey.
- * @param endRowKey End RowKey.
+ * @param tableName Table name.
+ * @param startRowKey Start RowKey.
+ * @param endRowKey End RowKey.
+ * @param queryTimeout Phoenix query timeout.
* @return The list of region locations that cover the startRowKey and endRowKey key boundary.
* @throws SQLException If fails to retrieve region locations.
*/
public List<HRegionLocation> getTableRegions(byte[] tableName, byte[] startRowKey,
- byte[] endRowKey) throws SQLException;
+ byte[] endRowKey,
+ int queryTimeout) throws SQLException;
Review Comment:
Sounds good, will address this in the next commit
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1568487609
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -800,6 +799,8 @@ public List<HRegionLocation> getTableRegions(byte[] tableName, byte[] startRowKe
&& Bytes.compareTo(currentKey, endRowKey) >= 0) {
break;
}
+ throwErrorIfQueryTimedOut(startRowKey, endRowKey, maxQueryEndTime,
Review Comment:
One of the tests in PhoenixQueryTimeoutIT is already covering this. However, the way `BaseResultIterators.setForTestingSetTimeoutToMaxToLetQueryPassHere` is being used is causing issues. We will have to clean it up or separate the test using it to different class. Given that it is static, it has potential to run into issues while running any other test in parallel.
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#issuecomment-2084223757
Will merge this in sometime, need to confirm whether the patch can be directly applied on 5.2 branch.
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani merged PR #1880:
URL: https://github.com/apache/phoenix/pull/1880
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1569161511
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java:
##########
@@ -99,22 +99,27 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated
public TableDescriptor getTableDescriptor(byte[] tableName) throws SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[] row) throws SQLException;
- public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws SQLException;
+
+ public List<HRegionLocation> getAllTableRegions(byte[] tableName, int queryTimeout)
Review Comment:
I see, sounds good. If we maintain the old API contract, we still want it to figure out the timeout value from the config right?
Let me also mark the old API deprecated.
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#issuecomment-2070362062
@virajjasani One comment, other than that looks 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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1569150615
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java:
##########
@@ -99,22 +99,27 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated
public TableDescriptor getTableDescriptor(byte[] tableName) throws SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[] row) throws SQLException;
- public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws SQLException;
+
+ public List<HRegionLocation> getAllTableRegions(byte[] tableName, int queryTimeout)
Review Comment:
Sorry, it is TableResultIterator which broke when we modified its constructor. This was fixed via https://issues.apache.org/jira/browse/PHOENIX-7102
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1575134090
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java:
##########
@@ -99,22 +99,48 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated
public TableDescriptor getTableDescriptor(byte[] tableName) throws SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[] row) throws SQLException;
+
+ /**
+ * Retrieve the region metadata locations for all regions of the given table.
+ * This method is Deprecated. Use {@link #getAllTableRegions(byte[], int)} instead.
+ *
+ * @param tableName The table name.
+ * @return The list of table region locations.
+ * @throws SQLException If fails to retrieve region locations.
+ */
+ @Deprecated
public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws SQLException;
+ /**
+ * Retrieve the region metadata locations for all regions of the given table.
+ * The operation to retrieve the table region locations must be completed within
+ * the query timeout.
+ *
+ * @param tableName Table name.
+ * @param queryTimeout Phoenix query timeout.
+ * @return The list of region locations.
+ * @throws SQLException If fails to retrieve region locations.
+ */
+ public List<HRegionLocation> getAllTableRegions(byte[] tableName, int queryTimeout)
+ throws SQLException;
+
/**
* Retrieve table region locations that cover the startRowKey and endRowKey. The start key
* of the first region of the returned list must be less than or equal to startRowKey.
* The end key of the last region of the returned list must be greater than or equal to
- * endRowKey.
+ * endRowKey. The operation to retrieve the table region locations must be completed within
+ * the query timeout.
*
- * @param tableName Table name.
- * @param startRowKey Start RowKey.
- * @param endRowKey End RowKey.
+ * @param tableName Table name.
+ * @param startRowKey Start RowKey.
+ * @param endRowKey End RowKey.
+ * @param queryTimeout Phoenix query timeout.
* @return The list of region locations that cover the startRowKey and endRowKey key boundary.
* @throws SQLException If fails to retrieve region locations.
*/
public List<HRegionLocation> getTableRegions(byte[] tableName, byte[] startRowKey,
- byte[] endRowKey) throws SQLException;
+ byte[] endRowKey,
+ int queryTimeout) throws SQLException;
Review Comment:
We should keep the existing API for this too and mark it deprecated just like the other 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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1568489027
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -780,6 +775,10 @@ public List<HRegionLocation> getTableRegions(byte[] tableName, byte[] startRowKe
config.getInt(PHOENIX_GET_REGIONS_RETRIES, DEFAULT_PHOENIX_GET_REGIONS_RETRIES);
TableName table = TableName.valueOf(tableName);
byte[] currentKey = null;
+ final int queryTimeout = this.getProps().getInt(QueryServices.THREAD_TIMEOUT_MS_ATTRIB,
Review Comment:
Sounds good, 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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1569165278
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java:
##########
@@ -99,22 +99,27 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated
public TableDescriptor getTableDescriptor(byte[] tableName) throws SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[] row) throws SQLException;
- public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws SQLException;
+
+ public List<HRegionLocation> getAllTableRegions(byte[] tableName, int queryTimeout)
Review Comment:
That is even better. The old API can fetch the timeout from config instead of using Long.max. That would have saved you some test changes where you are doing exactly that.
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1565837798
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -780,6 +775,10 @@ public List<HRegionLocation> getTableRegions(byte[] tableName, byte[] startRowKe
config.getInt(PHOENIX_GET_REGIONS_RETRIES, DEFAULT_PHOENIX_GET_REGIONS_RETRIES);
TableName table = TableName.valueOf(tableName);
byte[] currentKey = null;
+ final int queryTimeout = this.getProps().getInt(QueryServices.THREAD_TIMEOUT_MS_ATTRIB,
Review Comment:
querytimeout can be overridden at the phoenix statement level. We need to get it from the statementcontext `context.getStatement().getQueryTimeoutInMillis();`
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1568487609
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -800,6 +799,8 @@ public List<HRegionLocation> getTableRegions(byte[] tableName, byte[] startRowKe
&& Bytes.compareTo(currentKey, endRowKey) >= 0) {
break;
}
+ throwErrorIfQueryTimedOut(startRowKey, endRowKey, maxQueryEndTime,
Review Comment:
One of the tests in PhoenixQueryTimeoutIT is already covering this even without having to split/salt the table. However, the way `BaseResultIterators.setForTestingSetTimeoutToMaxToLetQueryPassHere` is being used is causing issues. We will have to clean it up or separate the test using it to different class. Given that it is static, it has potential to run into issues while running any other test in parallel.
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1569129093
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java:
##########
@@ -99,22 +99,27 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated
public TableDescriptor getTableDescriptor(byte[] tableName) throws SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[] row) throws SQLException;
- public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws SQLException;
+
+ public List<HRegionLocation> getAllTableRegions(byte[] tableName, int queryTimeout)
Review Comment:
The clients using the API either have StatementContext object or they have Config object so either way they need to provide timeout anyway.
If we keep the API as is, anyone would only use that directly even though they might have PhoenixStatement level overridden timeout. So using public API without timeout is not good practice anyway right?
For ScanningResultIterator, the constructor has timeout param and the clients are either providing Long.MAX_VALUE or Context level overridden timeout but the pattern is same i.e. clients using the APIs are providing timeout.
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1569113972
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java:
##########
@@ -99,22 +99,27 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated
public TableDescriptor getTableDescriptor(byte[] tableName) throws SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[] row) throws SQLException;
- public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws SQLException;
+
+ public List<HRegionLocation> getAllTableRegions(byte[] tableName, int queryTimeout)
Review Comment:
I would recommend not changing the public API but add a new API. We had similar problems in the past when we introduced a timeout in ScanningResultIterator. The existing APIs can send values like LONG.max.
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1563401481
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -739,8 +739,6 @@ public byte[] getNextRegionStartKey(HRegionLocation regionLocation, byte[] curre
&& !Bytes.equals(regionLocation.getRegion().getEndKey(), HConstants.EMPTY_END_ROW);
if (conditionOne || conditionTwo) {
GLOBAL_HBASE_COUNTER_METADATA_INCONSISTENCY.increment();
- String regionNameString =
- new String(regionLocation.getRegion().getRegionName(), StandardCharsets.UTF_8);
LOGGER.error(
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1565842537
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -800,6 +799,8 @@ public List<HRegionLocation> getTableRegions(byte[] tableName, byte[] startRowKe
&& Bytes.compareTo(currentKey, endRowKey) >= 0) {
break;
}
+ throwErrorIfQueryTimedOut(startRowKey, endRowKey, maxQueryEndTime,
Review Comment:
I think we can test this by setting a low value for a query timeout and creating a table with multiple regions either through salting or split points. PhoenixQueryTimeoutIT seems like a good candidate for this 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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1569169775
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java:
##########
@@ -99,22 +99,27 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated
public TableDescriptor getTableDescriptor(byte[] tableName) throws SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[] row) throws SQLException;
- public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws SQLException;
+
+ public List<HRegionLocation> getAllTableRegions(byte[] tableName, int queryTimeout)
Review Comment:
I think many of those test changes will be required because many of them don't have real config as such, we are either mocking or using constant. But yes, IT tests can use old API.
No problem, this can be taken care of in general. Though we have this test recently written (this month) that is getting stuck and not moving forward. This seems bit of a concern, either for using static test method directly on BaseResultIterator is becoming problematic or EnvironmentEdgeManager is causing troubles. Either way, the test might be blocking the whole build.
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1569151088
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java:
##########
@@ -99,22 +99,27 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated
public TableDescriptor getTableDescriptor(byte[] tableName) throws SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[] row) throws SQLException;
- public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws SQLException;
+
+ public List<HRegionLocation> getAllTableRegions(byte[] tableName, int queryTimeout)
Review Comment:
The point is we need to maintain API compatibility
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1569169775
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java:
##########
@@ -99,22 +99,27 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated
public TableDescriptor getTableDescriptor(byte[] tableName) throws SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[] row) throws SQLException;
- public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws SQLException;
+
+ public List<HRegionLocation> getAllTableRegions(byte[] tableName, int queryTimeout)
Review Comment:
I think many of those test changes will be required because many of them don't have real config as such, we are either mocking or using constant.
No problem, this can be taken care of in general. Though we have this test recently written (this month) that is getting stuck and not moving forward. This seems bit of a concern, either for using static test method directly on BaseResultIterator is becoming problematic or EnvironmentEdgeManager is causing troubles. Either way, the test might be blocking the whole build.
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#issuecomment-2068233405
The latest build: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1880/7/
--
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
Re: [PR] PHOENIX-7306 Metadata lookup should be permitted only within query timeout [phoenix]
Posted by "kadirozde (via GitHub)" <gi...@apache.org>.
kadirozde commented on code in PR #1880:
URL: https://github.com/apache/phoenix/pull/1880#discussion_r1563381196
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -739,8 +739,6 @@ public byte[] getNextRegionStartKey(HRegionLocation regionLocation, byte[] curre
&& !Bytes.equals(regionLocation.getRegion().getEndKey(), HConstants.EMPTY_END_ROW);
if (conditionOne || conditionTwo) {
GLOBAL_HBASE_COUNTER_METADATA_INCONSISTENCY.increment();
- String regionNameString =
- new String(regionLocation.getRegion().getRegionName(), StandardCharsets.UTF_8);
LOGGER.error(
Review Comment:
This should be warning.
--
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