[GitHub] [tomcat] toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling
toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-473394845 Thanks for taking the time to look at my changes! I totally missed the code dealing with dynamic reconfiguration through JMX. I changed my code to keep the old behaviour (pool cleaner is only started when timeBetweenEvictionRunsMillis > 0) and added documentation to maxAge pointing out that the age of idle connections will only be checked if both maxAge and timeBetweenEvictionRunsMillis are set. I also adjusted ConnectionPool#checkPoolConfiguration() to set timeBetweenEvictionRunsMillis=maxAge in case that timeBetweenEvictionRunsMillis>maxAge (because otherwise the maxAge cannot be guaranteed). setMaxAge() now calls both checkPoolConfiguration() and a new poolCleanerAttributeUpdated() method where I moved all the duplicated "start/stop pool cleaner if necessary" code to. 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] toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling
toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-473018240 I fixed the missing calls to PooledConnection#validate(int). While reading the code I was kind of unsure whether I should call PooledConnection#setTimestamp(long) when reconnecting an idle connection. Looking at the code it seems that the timestamp should only be updated when user-initiated (like borrow or return) operations are executed on the PooledConnection, not when (possibly internal) operations are executed on the underlying JDBC Connection - right ? The JavaDoc on setTimestamp() is kind of fuzzy and does not explain what kind of 'operations' are meant. 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] toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling
toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-472791041 Good catch, I will fix this. 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] toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling
toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-471474038 Sorry for not getting back any earlier. I did not touch the existing code responsible for discarding idle connections or the creation of new connections, so if your statement "if you have 0 active, then you have at least minIdle" used to be true in the past, it still holds now. The only change to the previous behaviour that *might* be perceived by a user is that busy connections returned to the pool that also exceeded maxAge are not getting discarded anymore but instead only the underlying JDBC connection gets recreated. This is only visible to a user if the pool is not being used again right away, otherwise a new connection would've been created and the change would be imperceivable. 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] toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling
toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-469229642 Could you please elaborate what you meant by your reference to the "maxIdle" and "initialSize" options ? How do they relate to the "maxAge" one in your opinion ? 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
