You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/06/21 21:50:54 UTC
[GitHub] [guacamole-client] mike-jumper opened a new pull request #622: GUACAMOLE-641: Correct regressions in custom pooled datasource behavior.
mike-jumper opened a new pull request #622:
URL: https://github.com/apache/guacamole-client/pull/622
The `DynamicallyAuthenticatedDataSource` introduced via [GUACAMOLE-641](https://issues.apache.org/jira/browse/GUACAMOLE-641) does not correctly set driver-specific JDBC properties like `allowMultiQueries`, resulting in failures for queries that depend on those properties. Additionally, the `PooledDataSource` constructor in use does not initialize internal values used by `PooledDataSource` to verify connections being returned to the pool, resulting in an eternally empty pool and new connections being created for each query.
This change corrects the above by:
1. Adding support for the same named injectable values as mybatis-guice's `PooledDataSourceProvider`.
2. Manually invoking `forceCloseAll()` within the constructor, which will initialize the required internals for `PooledDataSource` sanity checks.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] necouchman commented on a change in pull request #622: GUACAMOLE-641: Correct regressions in custom pooled datasource behavior.
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #622:
URL: https://github.com/apache/guacamole-client/pull/622#discussion_r657079935
##########
File path: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/DynamicallyAuthenticatedDataSource.java
##########
@@ -71,6 +80,77 @@ public Connection getConnection() throws SQLException {
});
+ // Force recalculation of expectedConnectionTypeCode. The
+ // PooledDataSource constructor accepting a single UnpooledDataSource
+ // will otherwise leave this value uninitialized, resulting in all
+ // connections failing to pass sanity checks and never being returned
+ // to the pool.
+ super.forceCloseAll();
+
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolPingConnectionsNotUsedFor(
+ @Named("mybatis.pooled.pingConnectionsNotUsedFor") int milliseconds) {
+ super.setPoolPingConnectionsNotUsedFor(milliseconds);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolPingEnabled(@Named("mybatis.pooled.pingEnabled") boolean poolPingEnabled) {
+ super.setPoolPingEnabled(poolPingEnabled);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolPingQuery(@Named("mybatis.pooled.pingQuery") String poolPingQuery) {
+ super.setPoolPingQuery(poolPingQuery);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolTimeToWait(@Named("mybatis.pooled.timeToWait") int poolTimeToWait) {
+ super.setPoolTimeToWait(poolTimeToWait);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolMaximumCheckoutTime(
+ @Named("mybatis.pooled.maximumCheckoutTime") int poolMaximumCheckoutTime) {
+ super.setPoolMaximumCheckoutTime(poolMaximumCheckoutTime);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolMaximumIdleConnections(
+ @Named("mybatis.pooled.maximumIdleConnections") int poolMaximumIdleConnections) {
+ super.setPoolMaximumIdleConnections(poolMaximumIdleConnections);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolMaximumActiveConnections(
+ @Named("mybatis.pooled.maximumActiveConnections") int poolMaximumActiveConnections) {
+ super.setPoolMaximumActiveConnections(poolMaximumActiveConnections);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setDriverProperties(@Named("JDBC.driverProperties") Properties driverProps) {
Review comment:
Are all of the other properties - like time zone, SSL options, etc. - handled, here? Or do they also need to be specified in this list?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #622: GUACAMOLE-641: Correct regressions in custom pooled datasource behavior.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #622:
URL: https://github.com/apache/guacamole-client/pull/622#discussion_r657387258
##########
File path: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/DynamicallyAuthenticatedDataSource.java
##########
@@ -71,6 +80,77 @@ public Connection getConnection() throws SQLException {
});
+ // Force recalculation of expectedConnectionTypeCode. The
+ // PooledDataSource constructor accepting a single UnpooledDataSource
+ // will otherwise leave this value uninitialized, resulting in all
+ // connections failing to pass sanity checks and never being returned
+ // to the pool.
+ super.forceCloseAll();
+
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolPingConnectionsNotUsedFor(
+ @Named("mybatis.pooled.pingConnectionsNotUsedFor") int milliseconds) {
+ super.setPoolPingConnectionsNotUsedFor(milliseconds);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolPingEnabled(@Named("mybatis.pooled.pingEnabled") boolean poolPingEnabled) {
+ super.setPoolPingEnabled(poolPingEnabled);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolPingQuery(@Named("mybatis.pooled.pingQuery") String poolPingQuery) {
+ super.setPoolPingQuery(poolPingQuery);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolTimeToWait(@Named("mybatis.pooled.timeToWait") int poolTimeToWait) {
+ super.setPoolTimeToWait(poolTimeToWait);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolMaximumCheckoutTime(
+ @Named("mybatis.pooled.maximumCheckoutTime") int poolMaximumCheckoutTime) {
+ super.setPoolMaximumCheckoutTime(poolMaximumCheckoutTime);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolMaximumIdleConnections(
+ @Named("mybatis.pooled.maximumIdleConnections") int poolMaximumIdleConnections) {
+ super.setPoolMaximumIdleConnections(poolMaximumIdleConnections);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setPoolMaximumActiveConnections(
+ @Named("mybatis.pooled.maximumActiveConnections") int poolMaximumActiveConnections) {
+ super.setPoolMaximumActiveConnections(poolMaximumActiveConnections);
+ }
+
+ @Override
+ @Inject(optional=true)
+ public void setDriverProperties(@Named("JDBC.driverProperties") Properties driverProps) {
Review comment:
They're all handled here, yes. The key things that need explicit injection are those mentioned in the `PooledDataSourceProvider` class from mybatis-guice:
https://github.com/mybatis/guice/blob/mybatis-guice-3.10/src/main/java/org/mybatis/guice/datasource/builtin/PooledDataSourceProvider.java
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] necouchman merged pull request #622: GUACAMOLE-641: Correct regressions in custom pooled datasource behavior.
Posted by GitBox <gi...@apache.org>.
necouchman merged pull request #622:
URL: https://github.com/apache/guacamole-client/pull/622
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org