dbwong commented on code in PR #1459:
URL: https://github.com/apache/phoenix/pull/1459#discussion_r915680872
##########
phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java:
##########
@@ -1099,6 +1100,53 @@ public void testGetConnectionsThrottledForSameUrl()
throws Exception {
assertEquals(maxConnections, connections.size());
}
+ @Test
+ public void testGetConnectionsFailedCounter() throws Exception {
+ int attemptedPhoenixConnections = 7;
+ //3 Failed connections and 1 throttled connection
+ int maxConnections = attemptedPhoenixConnections - 4;
+ List<Connection> connections = Lists.newArrayList();
+ String zkQuorum = "localhost:" +
getUtility().getZkCluster().getClientPort();
+ String url = PhoenixRuntime.JDBC_PROTOCOL +
PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR + zkQuorum +
+ ':' + CUSTOM_URL_STRING + '=' + "FailedCounterTest";
+ Properties props = new Properties();
+
props.setProperty(QueryServices.CLIENT_CONNECTION_MAX_ALLOWED_CONNECTIONS,
Integer.toString(maxConnections));
+ Properties props1 = new Properties(props);
+ props1.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB,
Integer.toString(-1));
+
+ GLOBAL_QUERY_SERVICES_COUNTER.getMetric().reset();
+ GLOBAL_PHOENIX_CONNECTIONS_ATTEMPTED_COUNTER.getMetric().reset();
+ GLOBAL_PHOENIX_CONNECTIONS_THROTTLED_COUNTER.getMetric().reset();
+ GLOBAL_FAILED_PHOENIX_CONNECTIONS.getMetric().reset();
+ try {
+ for (int i = 0; i < attemptedPhoenixConnections; i++) {
+ try {
+ if (i % 3 == 0) {
+ connections.add(DriverManager.getConnection(url,
props1));
+ } else {
+ connections.add(DriverManager.getConnection(url,
props));
+ }
+ } catch (SQLException se) {
+ if (i % 3 == 0) {
+
assertEquals(SQLExceptionCode.INVALID_SCN.getErrorCode(), se.getErrorCode());
+ } else {
+
assertEquals(SQLExceptionCode.NEW_CONNECTION_THROTTLED.getErrorCode(),
se.getErrorCode());
+ }
+ }
+ }
+ } finally {
+ for (Connection c : connections) {
+ c.close();
+ }
+ }
+ assertEquals(1, GLOBAL_QUERY_SERVICES_COUNTER.getMetric().getValue());
+ assertEquals(1,
GLOBAL_PHOENIX_CONNECTIONS_THROTTLED_COUNTER.getMetric().getValue());
+ assertEquals(3,
GLOBAL_FAILED_PHOENIX_CONNECTIONS.getMetric().getValue());
+ assertTrue("Not all connections were attempted!",
+ attemptedPhoenixConnections <=
GLOBAL_PHOENIX_CONNECTIONS_ATTEMPTED_COUNTER.getMetric().getValue());
Review Comment:
Shouldn't we have equality here?
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java:
##########
@@ -254,163 +255,170 @@ private PhoenixConnection(ConnectionQueryServices
services, String url,
Properties info, PMetaData metaData, MutationState mutationState,
boolean isDescVarLengthRowKeyUpgrade, boolean isRunningUpgrade,
boolean buildingIndex, boolean isInternalConnection) throws
SQLException {
- GLOBAL_PHOENIX_CONNECTIONS_ATTEMPTED_COUNTER.increment();
- this.url = url;
- this.isDescVarLengthRowKeyUpgrade = isDescVarLengthRowKeyUpgrade;
- this.isInternalConnection = isInternalConnection;
-
- // Filter user provided properties based on property policy, if
- // provided and QueryServices.PROPERTY_POLICY_PROVIDER_ENABLED is true
- if
(Boolean.valueOf(info.getProperty(QueryServices.PROPERTY_POLICY_PROVIDER_ENABLED,
-
String.valueOf(QueryServicesOptions.DEFAULT_PROPERTY_POLICY_PROVIDER_ENABLED))))
{
- PropertyPolicyProvider.getPropertyPolicy().evaluate(info);
- }
-
- // Copy so client cannot change
- this.info = PropertiesUtil.deepCopy(info);
- final PName tenantId = JDBCUtil.getTenantId(url, info);
- if (this.info.isEmpty() && tenantId == null) {
- this.services = services;
- } else {
- // Create child services keyed by tenantId to track resource usage
- // for
- // a tenantId for all connections on this JVM.
- if (tenantId != null) {
- services = services.getChildQueryServices(tenantId
- .getBytesPtr());
+ try {
+ GLOBAL_PHOENIX_CONNECTIONS_ATTEMPTED_COUNTER.increment();
+ this.url = url;
+ this.isDescVarLengthRowKeyUpgrade = isDescVarLengthRowKeyUpgrade;
+ this.isInternalConnection = isInternalConnection;
+
+ // Filter user provided properties based on property policy, if
+ // provided and QueryServices.PROPERTY_POLICY_PROVIDER_ENABLED is
true
+ if
(Boolean.valueOf(info.getProperty(QueryServices.PROPERTY_POLICY_PROVIDER_ENABLED,
+
String.valueOf(QueryServicesOptions.DEFAULT_PROPERTY_POLICY_PROVIDER_ENABLED))))
{
+ PropertyPolicyProvider.getPropertyPolicy().evaluate(info);
}
- ReadOnlyProps currentProps = services.getProps();
- final ReadOnlyProps augmentedProps = currentProps
- .addAll(filterKnownNonProperties(this.info));
- this.services = augmentedProps == currentProps ? services
- : new DelegateConnectionQueryServices(services) {
- @Override
- public ReadOnlyProps getProps() {
- return augmentedProps;
+
+ // Copy so client cannot change
+ this.info = PropertiesUtil.deepCopy(info);
+ final PName tenantId = JDBCUtil.getTenantId(url, info);
+ if (this.info.isEmpty() && tenantId == null) {
+ this.services = services;
+ } else {
+ // Create child services keyed by tenantId to track resource
usage
+ // for
+ // a tenantId for all connections on this JVM.
+ if (tenantId != null) {
+ services = services.getChildQueryServices(tenantId
+ .getBytesPtr());
}
- };
- }
+ ReadOnlyProps currentProps = services.getProps();
+ final ReadOnlyProps augmentedProps = currentProps
+ .addAll(filterKnownNonProperties(this.info));
+ this.services = augmentedProps == currentProps ? services
+ : new DelegateConnectionQueryServices(services) {
+ @Override
+ public ReadOnlyProps getProps() {
+ return augmentedProps;
+ }
+ };
+ }
- Long scnParam = JDBCUtil.getCurrentSCN(url, this.info);
- checkScn(scnParam);
- Long buildIndexAtParam = JDBCUtil.getBuildIndexSCN(url, this.info);
- checkBuildIndexAt(buildIndexAtParam);
- checkScnAndBuildIndexAtEquality(scnParam, buildIndexAtParam);
+ Long scnParam = JDBCUtil.getCurrentSCN(url, this.info);
+ checkScn(scnParam);
+ Long buildIndexAtParam = JDBCUtil.getBuildIndexSCN(url, this.info);
+ checkBuildIndexAt(buildIndexAtParam);
+ checkScnAndBuildIndexAtEquality(scnParam, buildIndexAtParam);
+
+ this.scn = scnParam != null ? scnParam : buildIndexAtParam;
+ this.buildingIndex = buildingIndex || buildIndexAtParam != null;
+ this.isAutoFlush = this.services.getProps().getBoolean(
+ QueryServices.TRANSACTIONS_ENABLED,
+ QueryServicesOptions.DEFAULT_TRANSACTIONS_ENABLED)
+ && this.services.getProps().getBoolean(
+ QueryServices.AUTO_FLUSH_ATTRIB,
+ QueryServicesOptions.DEFAULT_AUTO_FLUSH);
+ this.isAutoCommit = JDBCUtil.getAutoCommit(
+ url,
+ this.info,
+ this.services.getProps().getBoolean(
+ QueryServices.AUTO_COMMIT_ATTRIB,
+ QueryServicesOptions.DEFAULT_AUTO_COMMIT));
+ this.consistency = JDBCUtil.getConsistencyLevel(
+ url,
+ this.info,
+
this.services.getProps().get(QueryServices.CONSISTENCY_ATTRIB,
+ QueryServicesOptions.DEFAULT_CONSISTENCY_LEVEL));
+ // currently we are not resolving schema set through property, so
if
+ // schema doesn't exists ,connection will not fail
+ // but queries may fail
+ this.schema = JDBCUtil.getSchema(
+ url,
+ this.info,
+ this.services.getProps().get(QueryServices.SCHEMA_ATTRIB,
+ QueryServicesOptions.DEFAULT_SCHEMA));
+ this.tenantId = tenantId;
+ this.mutateBatchSize = JDBCUtil.getMutateBatchSize(url, this.info,
+ this.services.getProps());
+ this.mutateBatchSizeBytes = JDBCUtil.getMutateBatchSizeBytes(url,
+ this.info, this.services.getProps());
+ datePattern = this.services.getProps().get(
+ QueryServices.DATE_FORMAT_ATTRIB,
DateUtil.DEFAULT_DATE_FORMAT);
+ timePattern = this.services.getProps().get(
+ QueryServices.TIME_FORMAT_ATTRIB,
DateUtil.DEFAULT_TIME_FORMAT);
+ timestampPattern = this.services.getProps().get(
+ QueryServices.TIMESTAMP_FORMAT_ATTRIB,
+ DateUtil.DEFAULT_TIMESTAMP_FORMAT);
+ String numberPattern = this.services.getProps().get(
+ QueryServices.NUMBER_FORMAT_ATTRIB,
+ NumberUtil.DEFAULT_NUMBER_FORMAT);
+ int maxSize = this.services.getProps().getInt(
+ QueryServices.MAX_MUTATION_SIZE_ATTRIB,
+ QueryServicesOptions.DEFAULT_MAX_MUTATION_SIZE);
+ long maxSizeBytes = this.services.getProps().getLong(
+ QueryServices.MAX_MUTATION_SIZE_BYTES_ATTRIB,
+ QueryServicesOptions.DEFAULT_MAX_MUTATION_SIZE_BYTES);
+ String timeZoneID =
this.services.getProps().get(QueryServices.DATE_FORMAT_TIMEZONE_ATTRIB,
+ DateUtil.DEFAULT_TIME_ZONE_ID);
+ Format dateFormat = DateUtil.getDateFormatter(datePattern,
timeZoneID);
+ Format timeFormat = DateUtil.getDateFormatter(timePattern,
timeZoneID);
+ Format timestampFormat =
DateUtil.getDateFormatter(timestampPattern, timeZoneID);
+ formatters.put(PDate.INSTANCE, dateFormat);
+ formatters.put(PTime.INSTANCE, timeFormat);
+ formatters.put(PTimestamp.INSTANCE, timestampFormat);
+ formatters.put(PUnsignedDate.INSTANCE, dateFormat);
+ formatters.put(PUnsignedTime.INSTANCE, timeFormat);
+ formatters.put(PUnsignedTimestamp.INSTANCE, timestampFormat);
+ formatters.put(PDecimal.INSTANCE,
+ FunctionArgumentType.NUMERIC.getFormatter(numberPattern));
+ formatters.put(PVarbinary.INSTANCE, VarBinaryFormatter.INSTANCE);
+ // We do not limit the metaData on a connection less than the
global
+ // one,
+ // as there's not much that will be cached here.
+ Pruner pruner = new Pruner() {
- this.scn = scnParam != null ? scnParam : buildIndexAtParam;
- this.buildingIndex = buildingIndex || buildIndexAtParam != null;
- this.isAutoFlush = this.services.getProps().getBoolean(
- QueryServices.TRANSACTIONS_ENABLED,
- QueryServicesOptions.DEFAULT_TRANSACTIONS_ENABLED)
- && this.services.getProps().getBoolean(
- QueryServices.AUTO_FLUSH_ATTRIB,
- QueryServicesOptions.DEFAULT_AUTO_FLUSH);
- this.isAutoCommit = JDBCUtil.getAutoCommit(
- url,
- this.info,
- this.services.getProps().getBoolean(
- QueryServices.AUTO_COMMIT_ATTRIB,
- QueryServicesOptions.DEFAULT_AUTO_COMMIT));
- this.consistency = JDBCUtil.getConsistencyLevel(
- url,
- this.info,
- this.services.getProps().get(QueryServices.CONSISTENCY_ATTRIB,
- QueryServicesOptions.DEFAULT_CONSISTENCY_LEVEL));
- // currently we are not resolving schema set through property, so if
- // schema doesn't exists ,connection will not fail
- // but queries may fail
- this.schema = JDBCUtil.getSchema(
- url,
- this.info,
- this.services.getProps().get(QueryServices.SCHEMA_ATTRIB,
- QueryServicesOptions.DEFAULT_SCHEMA));
- this.tenantId = tenantId;
- this.mutateBatchSize = JDBCUtil.getMutateBatchSize(url, this.info,
- this.services.getProps());
- this.mutateBatchSizeBytes = JDBCUtil.getMutateBatchSizeBytes(url,
- this.info, this.services.getProps());
- datePattern = this.services.getProps().get(
- QueryServices.DATE_FORMAT_ATTRIB,
DateUtil.DEFAULT_DATE_FORMAT);
- timePattern = this.services.getProps().get(
- QueryServices.TIME_FORMAT_ATTRIB,
DateUtil.DEFAULT_TIME_FORMAT);
- timestampPattern = this.services.getProps().get(
- QueryServices.TIMESTAMP_FORMAT_ATTRIB,
- DateUtil.DEFAULT_TIMESTAMP_FORMAT);
- String numberPattern = this.services.getProps().get(
- QueryServices.NUMBER_FORMAT_ATTRIB,
- NumberUtil.DEFAULT_NUMBER_FORMAT);
- int maxSize = this.services.getProps().getInt(
- QueryServices.MAX_MUTATION_SIZE_ATTRIB,
- QueryServicesOptions.DEFAULT_MAX_MUTATION_SIZE);
- long maxSizeBytes = this.services.getProps().getLong(
- QueryServices.MAX_MUTATION_SIZE_BYTES_ATTRIB,
- QueryServicesOptions.DEFAULT_MAX_MUTATION_SIZE_BYTES);
- String timeZoneID =
this.services.getProps().get(QueryServices.DATE_FORMAT_TIMEZONE_ATTRIB,
- DateUtil.DEFAULT_TIME_ZONE_ID);
- Format dateFormat = DateUtil.getDateFormatter(datePattern, timeZoneID);
- Format timeFormat = DateUtil.getDateFormatter(timePattern, timeZoneID);
- Format timestampFormat = DateUtil.getDateFormatter(timestampPattern,
timeZoneID);
- formatters.put(PDate.INSTANCE, dateFormat);
- formatters.put(PTime.INSTANCE, timeFormat);
- formatters.put(PTimestamp.INSTANCE, timestampFormat);
- formatters.put(PUnsignedDate.INSTANCE, dateFormat);
- formatters.put(PUnsignedTime.INSTANCE, timeFormat);
- formatters.put(PUnsignedTimestamp.INSTANCE, timestampFormat);
- formatters.put(PDecimal.INSTANCE,
- FunctionArgumentType.NUMERIC.getFormatter(numberPattern));
- formatters.put(PVarbinary.INSTANCE, VarBinaryFormatter.INSTANCE);
- // We do not limit the metaData on a connection less than the global
- // one,
- // as there's not much that will be cached here.
- Pruner pruner = new Pruner() {
+ @Override
+ public boolean prune(PTable table) {
+ long maxTimestamp = scn == null ?
HConstants.LATEST_TIMESTAMP
+ : scn;
+ return (table.getType() != PTableType.SYSTEM && (table
+ .getTimeStamp() >= maxTimestamp ||
(table.getTenantId() != null && !Objects
+ .equal(tenantId, table.getTenantId()))));
+ }
- @Override
- public boolean prune(PTable table) {
- long maxTimestamp = scn == null ? HConstants.LATEST_TIMESTAMP
- : scn;
- return (table.getType() != PTableType.SYSTEM && (table
- .getTimeStamp() >= maxTimestamp ||
(table.getTenantId() != null && !Objects
- .equal(tenantId, table.getTenantId()))));
+ @Override
+ public boolean prune(PFunction function) {
+ long maxTimestamp = scn == null ?
HConstants.LATEST_TIMESTAMP
+ : scn;
+ return (function.getTimeStamp() >= maxTimestamp ||
(function
+ .getTenantId() != null && !Objects.equal(tenantId,
+ function.getTenantId())));
+ }
+ };
+ this.logLevel =
LogLevel.valueOf(this.services.getProps().get(QueryServices.LOG_LEVEL,
+ QueryServicesOptions.DEFAULT_LOGGING_LEVEL));
+ this.auditLogLevel =
LogLevel.valueOf(this.services.getProps().get(QueryServices.AUDIT_LOG_LEVEL,
+ QueryServicesOptions.DEFAULT_AUDIT_LOGGING_LEVEL));
+ this.isRequestLevelMetricsEnabled =
JDBCUtil.isCollectingRequestLevelMetricsEnabled(url, info,
+ this.services.getProps());
+ this.mutationState = mutationState == null ?
newMutationState(maxSize,
+ maxSizeBytes) : new MutationState(mutationState, this);
+ this.metaData = metaData;
+ this.metaData.pruneTables(pruner);
+ this.metaData.pruneFunctions(pruner);
+ this.services.addConnection(this);
+
+ // setup tracing, if its enabled
+ this.sampler = Tracing.getConfiguredSampler(this);
+ this.customTracingAnnotations =
getImmutableCustomTracingAnnotations();
+ this.scannerQueue = new LinkedBlockingQueue<>();
+ this.tableResultIteratorFactory = new
DefaultTableResultIteratorFactory();
+ this.isRunningUpgrade = isRunningUpgrade;
+
+ this.logSamplingRate =
Double.parseDouble(this.services.getProps().get(QueryServices.LOG_SAMPLE_RATE,
+ QueryServicesOptions.DEFAULT_LOG_SAMPLE_RATE));
+ if (isInternalConnection) {
+ GLOBAL_OPEN_INTERNAL_PHOENIX_CONNECTIONS.increment();
+ } else {
+ GLOBAL_OPEN_PHOENIX_CONNECTIONS.increment();
}
-
- @Override
- public boolean prune(PFunction function) {
- long maxTimestamp = scn == null ? HConstants.LATEST_TIMESTAMP
- : scn;
- return (function.getTimeStamp() >= maxTimestamp || (function
- .getTenantId() != null && !Objects.equal(tenantId,
- function.getTenantId())));
+ this.sourceOfOperation =
+
this.services.getProps().get(QueryServices.SOURCE_OPERATION_ATTRIB, null);
+ } catch (SQLException sqlException) {
+ if (!isInternalConnection && sqlException.getErrorCode() !=
SQLExceptionCode.NEW_CONNECTION_THROTTLED.getErrorCode()) {
+ GLOBAL_FAILED_PHOENIX_CONNECTIONS.increment();
}
- };
- this.logLevel=
LogLevel.valueOf(this.services.getProps().get(QueryServices.LOG_LEVEL,
- QueryServicesOptions.DEFAULT_LOGGING_LEVEL));
- this.auditLogLevel=
LogLevel.valueOf(this.services.getProps().get(QueryServices.AUDIT_LOG_LEVEL,
- QueryServicesOptions.DEFAULT_AUDIT_LOGGING_LEVEL));
- this.isRequestLevelMetricsEnabled =
JDBCUtil.isCollectingRequestLevelMetricsEnabled(url, info,
- this.services.getProps());
- this.mutationState = mutationState == null ? newMutationState(maxSize,
- maxSizeBytes) : new MutationState(mutationState, this);
- this.metaData = metaData;
- this.metaData.pruneTables(pruner);
- this.metaData.pruneFunctions(pruner);
- this.services.addConnection(this);
-
- // setup tracing, if its enabled
- this.sampler = Tracing.getConfiguredSampler(this);
- this.customTracingAnnotations = getImmutableCustomTracingAnnotations();
- this.scannerQueue = new LinkedBlockingQueue<>();
- this.tableResultIteratorFactory = new
DefaultTableResultIteratorFactory();
- this.isRunningUpgrade = isRunningUpgrade;
-
- this.logSamplingRate =
Double.parseDouble(this.services.getProps().get(QueryServices.LOG_SAMPLE_RATE,
- QueryServicesOptions.DEFAULT_LOG_SAMPLE_RATE));
- if(isInternalConnection) {
- GLOBAL_OPEN_INTERNAL_PHOENIX_CONNECTIONS.increment();
- } else {
- GLOBAL_OPEN_PHOENIX_CONNECTIONS.increment();
+ throw sqlException;
}
Review Comment:
What about non-SQLExceptions that we might accidentally throw?
--
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]
