[GitHub] [tomcat] davoustp commented on issue #170: Connections validated without explicit validation query leave a transaction open
davoustp commented on issue #170: Connections validated without explicit validation query leave a transaction open URL: https://github.com/apache/tomcat/pull/170#issuecomment-536993033 Thx Mark! 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] davoustp commented on issue #170: Connections validated without explicit validation query leave a transaction open
davoustp commented on issue #170: Connections validated without explicit validation query leave a transaction open URL: https://github.com/apache/tomcat/pull/170#issuecomment-535816682 Hi, any idea if and when this fix will be merged into a Tomcat release? Thx 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] davoustp commented on issue #170: Connections validated without explicit validation query leave a transaction open
davoustp commented on issue #170: Connections validated without explicit validation query leave a transaction open URL: https://github.com/apache/tomcat/pull/170#issuecomment-503435774 Done. Please let me know what you think. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] davoustp commented on issue #170: Connections validated without explicit validation query leave a transaction open
davoustp commented on issue #170: Connections validated without explicit validation query leave a transaction open URL: https://github.com/apache/tomcat/pull/170#issuecomment-503323483 Agreed, the driver is expected to be as thin and stateless as possible. Indeed, using a flag to track if the transaction has been cleared or not is and performing the rollback in the latter case in a `finally` block is a much better solution as it would simplify the code and, even more importantly, it will also cover the corner cases which are not taken care of with by my initial commit (unchecked exceptions, or `Error`s). I'll do this for both code paths (with and without a validation query) and commit the changes to the PR so we can have another look to check if there are some remaining issues. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] davoustp commented on issue #170: Connections validated without explicit validation query leave a transaction open
davoustp commented on issue #170: Connections validated without explicit validation query leave a transaction open URL: https://github.com/apache/tomcat/pull/170#issuecomment-503212478 You mean that `silentlyRollbackTransactionIfNeeded()` at line 532? There is no `finally` block in this `try` statement. The `try` block needs either a commit or rollback, depending on the returned value from `isValid(int)`, and we need to rollback whenever an `Exception` is raised (in the `catch` block). There is one `finally` in the other (existing) code block at line 560: this was already in place, so left it here. I believe that this is actually an issue: the `rollback` is issued even if the `try` block did not raise any exception and committed its changes successfully - meaning that, unless the driver knows that this is a no-op (which I doubt), it will issue an unnecessary query (therefore a round-trip) to the DB. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
