dengzhhu653 commented on code in PR #5661:
URL: https://github.com/apache/hive/pull/5661#discussion_r1987129354


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/HikariCPDataSourceProvider.java:
##########
@@ -90,7 +94,42 @@ public DataSource create(Configuration hdpConfig, int 
maxPoolSize) throws SQLExc
       config.addDataSourceProperty(kv.getKey(), kv.getValue());
     }
 
-    return new HikariDataSource(initMetrics(config));
+    return new HikariDataSource(initMetrics(config)) {
+      @Override
+      public Connection getConnection() throws SQLException {

Review Comment:
   Yes, the solution may not sound elegant at first sight, but it only adds the 
proxy around two connections from the secondary connection pool, the primary 
pool stays the same.
   > we don't really know what side effects may appear if we introduce this 
rollback and close logic since test coverage right now is probably zero.
   
   If an exception is happing on committing the transaction, then we know:
    1. At this point, the code reaches the end of transaction;
    2. The current operation(API call) should be failing.
   
   so back to the solution,  we free the connection to the pool and throw the 
original exception to the caller, this is an expected behavior based on what we 
know, and the ObjectStore will rollback the current transaction(the primary 
connection), it should clean the current context by design.
   
   This solution only affect the current ObjectStore, as an ObjectStore could 
be requested for other API calls after the issue occurs, we should ensure it do 
the right thing. I changed this 
https://github.com/dengzhhu653/hive/commit/7b4a66cf030e8570214341424bab258520abe919
 a bit, it shows an insert following by the failed one(get or insert)(rollback 
and close logic) can get a successful run.
   
   However the connection leak affects the whole service, it cannot serve all 
the requests until a restart, this is worst compared to only affecting the 
current ObjectStore.
       



-- 
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: gitbox-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to