shahrs87 commented on code in PR #1666:
URL: https://github.com/apache/phoenix/pull/1666#discussion_r1325108801
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -317,16 +331,83 @@ protected QueryPlan optimizeQuery(CompilableStatement
stmt) throws SQLException
protected PhoenixResultSet executeQuery(final CompilableStatement stmt,
final QueryLogger queryLogger)
throws SQLException {
- return executeQuery(stmt, true, queryLogger, false);
+ return executeQuery(stmt, true, queryLogger, false,
this.ddlTimestampValidationEnabled);
}
protected PhoenixResultSet executeQuery(final CompilableStatement stmt,
final QueryLogger queryLogger, boolean noCommit)
throws SQLException {
- return executeQuery(stmt, true, queryLogger, noCommit);
+ return executeQuery(stmt, true, queryLogger, noCommit,
this.ddlTimestampValidationEnabled);
+ }
+
+ /**
+ * Build a request for the validateLastDDLTimestamp RPC.
+ * @param tableRef
+ * @return
+ */
+ private RegionServerEndpointProtos.ValidateLastDDLTimestampRequest
getValidateDDLTimestampRequest(TableRef tableRef) {
+ RegionServerEndpointProtos.ValidateLastDDLTimestampRequest.Builder
requestBuilder
+ =
RegionServerEndpointProtos.ValidateLastDDLTimestampRequest.newBuilder();
+ RegionServerEndpointProtos.LastDDLTimestampRequest.Builder innerBuilder
+ =
RegionServerEndpointProtos.LastDDLTimestampRequest.newBuilder();
+
+ byte[] tenantIDBytes = this.connection.getTenantId() == null ?
HConstants.EMPTY_BYTE_ARRAY : this.connection.getTenantId().getBytes();
+ byte[] schemaBytes = tableRef.getTable().getSchemaName() == null ?
HConstants.EMPTY_BYTE_ARRAY : tableRef.getTable().getSchemaName().getBytes();
+
+ innerBuilder.setTenantId(ByteStringer.wrap(tenantIDBytes));
+ innerBuilder.setSchemaName(ByteStringer.wrap(schemaBytes));
+
innerBuilder.setTableName(ByteStringer.wrap(tableRef.getTable().getTableName().getBytes()));
+
innerBuilder.setLastDDLTimestamp(tableRef.getTable().getLastDDLTimestamp());
+ requestBuilder.addLastDDLTimestampRequests(innerBuilder);
+ return requestBuilder.build();
+ }
+
+ /**
+ * Verifies that table metadata in client cache is up-to-date with server.
+ * A random live region server is picked for invoking the RPC to validate
LastDDLTimestamp.
+ * Retry once if there was an error performing the RPC, otherwise throw
the Exception.
+ * @param tableRef
+ * @throws SQLException
+ */
+ private void validateDDLTimestamp(TableRef tableRef, boolean doRetry)
throws SQLException {
+
+ try (Admin admin = this.connection.getQueryServices().getAdmin()) {
+ // get all live region servers
+ List<ServerName> regionServers = new
ArrayList<>(admin.getRegionServers(true));
+ // pick one at random
+ ServerName regionServer =
regionServers.get(ThreadLocalRandom.current().nextInt(regionServers.size()));
+
+ LOGGER.debug("Sending DDL timestamp validation request for table
{} at regionserver {}",
Review Comment:
Lets log the tenant ID and schema name along with table name.
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -417,11 +504,22 @@ private PhoenixResultSet executeQuery(final
CompilableStatement stmt,
e.getSchemaName(),
e.getTableName(), true)
.wasUpdated()) {
//TODO we can log retry count and
error for debugging in LOG table
- return executeQuery(stmt, false,
queryLogger, noCommit);
+ return executeQuery(stmt, false,
queryLogger, noCommit, validateDDLTimestamp);
}
}
throw e;
- } catch (RuntimeException e) {
+ }
+ catch (SQLException e) {
+ // force update cache in case of
StaleMetadataCacheException and retry
Review Comment:
Trying to understand why we have retries here and also in
validateDDLTimestamp? Can we consolidate them just in 1 place? Preferably in
validateDDLTimestamp?
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -317,16 +331,83 @@ protected QueryPlan optimizeQuery(CompilableStatement
stmt) throws SQLException
protected PhoenixResultSet executeQuery(final CompilableStatement stmt,
final QueryLogger queryLogger)
throws SQLException {
- return executeQuery(stmt, true, queryLogger, false);
+ return executeQuery(stmt, true, queryLogger, false,
this.ddlTimestampValidationEnabled);
}
protected PhoenixResultSet executeQuery(final CompilableStatement stmt,
final QueryLogger queryLogger, boolean noCommit)
throws SQLException {
- return executeQuery(stmt, true, queryLogger, noCommit);
+ return executeQuery(stmt, true, queryLogger, noCommit,
this.ddlTimestampValidationEnabled);
+ }
+
+ /**
+ * Build a request for the validateLastDDLTimestamp RPC.
+ * @param tableRef
+ * @return
Review Comment:
return missing.
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -317,16 +331,83 @@ protected QueryPlan optimizeQuery(CompilableStatement
stmt) throws SQLException
protected PhoenixResultSet executeQuery(final CompilableStatement stmt,
final QueryLogger queryLogger)
throws SQLException {
- return executeQuery(stmt, true, queryLogger, false);
+ return executeQuery(stmt, true, queryLogger, false,
this.ddlTimestampValidationEnabled);
}
protected PhoenixResultSet executeQuery(final CompilableStatement stmt,
final QueryLogger queryLogger, boolean noCommit)
throws SQLException {
- return executeQuery(stmt, true, queryLogger, noCommit);
+ return executeQuery(stmt, true, queryLogger, noCommit,
this.ddlTimestampValidationEnabled);
+ }
+
+ /**
+ * Build a request for the validateLastDDLTimestamp RPC.
+ * @param tableRef
+ * @return
+ */
+ private RegionServerEndpointProtos.ValidateLastDDLTimestampRequest
getValidateDDLTimestampRequest(TableRef tableRef) {
+ RegionServerEndpointProtos.ValidateLastDDLTimestampRequest.Builder
requestBuilder
+ =
RegionServerEndpointProtos.ValidateLastDDLTimestampRequest.newBuilder();
+ RegionServerEndpointProtos.LastDDLTimestampRequest.Builder innerBuilder
+ =
RegionServerEndpointProtos.LastDDLTimestampRequest.newBuilder();
+
+ byte[] tenantIDBytes = this.connection.getTenantId() == null ?
HConstants.EMPTY_BYTE_ARRAY : this.connection.getTenantId().getBytes();
Review Comment:
Need to fix checkstyle errors? Line length is more than 100 characters.
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -317,16 +331,83 @@ protected QueryPlan optimizeQuery(CompilableStatement
stmt) throws SQLException
protected PhoenixResultSet executeQuery(final CompilableStatement stmt,
final QueryLogger queryLogger)
throws SQLException {
- return executeQuery(stmt, true, queryLogger, false);
+ return executeQuery(stmt, true, queryLogger, false,
this.ddlTimestampValidationEnabled);
Review Comment:
Do we need to pass this variable around even if it is defined at the class
level?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
