RE: Question about resetting datasources and changes to the BasicDataSource.close() method
-Original Message- From: Christopher Schultz [mailto:ch...@christopherschultz.net] Sent: Thursday, May 31, 2012 5:23 PM Yup: the solution is to just synchronize all the methods. If restart() is synchronized, it will only operate while other methods are not actively checking-out or checking-in connections. The big problem with the concept of a restart is that there could be connections that are currently checked- out during the call to recycle()... those need to be handled in some special way -- probably added to a queue that will be processed later to destroy the connections. That's what I will submit to commons-dbcp, then. On the one hand, synchronizing these methods should not be necessary because Java boolean values are defined to be 32-bit integers which feature atomic assignment (that is, no thread ever sees only 8-bits of the 32-bit value being assigned and 24-bits of the old value). On the other hand, threads are allowed to keep cached copies of certain data under certain conditions. Using the keyword 'volatile' /should/, in recent JVMs, make the use of 'synchronized' completely unnecessary, while older JVMs may even ignore 'volatile' making 'synchronized' mandatory. Use of 'synchronized' is the safest bet because it definitely causes a 'memory barrier' to be crossed in any version of JVM: that means that the thread is required to synchronize (perhaps a bad choice of words) its cache with main memory which means that all variables get the latest copies of the real data. Yep. That was my point too. What does it really matter anyway whether you are able to lock this in a getter. When you ask for the value, you are just asking for what is current. You have to protect yourself from non-initialized/null in any case. It just adds some unnecessary overhead ( to the JVM and brain ) to synchronize your getter when it doesn't mutate the data - unless I am missing something. --Brooke
Re: Question about resetting datasources and changes to the BasicDataSource.close() method
2012/5/30 Hedrick, Brooke - 43 brooke.hedr...@rainhail.com: (...) Next, I looked at the new datasource org.apache.tomcat.jdbc.pool.DataSource. I have not found any methods to close/reset the pools and the JMX attributes are readonly. This prevents us both from resetting and resizing our connection pools. By the way: fix bug53254/bug (rev1340160/rev): Add in the ability to purge connections from the pool (fhanik) /fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53254 It will be in Tomcat JDBC pool in 7.0.28. Best regards, Konstantin Kolinko - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Question about resetting datasources and changes to the BasicDataSource.close() method
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Brooke, On 6/1/12 11:54 AM, Hedrick, Brooke - 43 wrote: -Original Message- From: Christopher Schultz [mailto:ch...@christopherschultz.net] Sent: Thursday, May 31, 2012 5:23 PM That's what I will submit to commons-dbcp, then. On the one hand, synchronizing these methods should not be necessary because Java boolean values are defined to be 32-bit integers which feature atomic assignment (that is, no thread ever sees only 8-bits of the 32-bit value being assigned and 24-bits of the old value). On the other hand, threads are allowed to keep cached copies of certain data under certain conditions. Using the keyword 'volatile' /should/, in recent JVMs, make the use of 'synchronized' completely unnecessary, while older JVMs may even ignore 'volatile' making 'synchronized' mandatory. Use of 'synchronized' is the safest bet because it definitely causes a 'memory barrier' to be crossed in any version of JVM: that means that the thread is required to synchronize (perhaps a bad choice of words) its cache with main memory which means that all variables get the latest copies of the real data. Yep. That was my point too. What does it really matter anyway whether you are able to lock this in a getter. When you ask for the value, you are just asking for what is current. You have to protect yourself from non-initialized/null in any case. It just adds some unnecessary overhead ( to the JVM and brain ) to synchronize your getter when it doesn't mutate the data - unless I am missing something. Locking this makes sure that the thread's local copy of whatever-you-are-getting is updated properly and not stale-cached. If you synchronize the setter, it is a very good idea to synchronize the getter too. - -chris -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk/I6xcACgkQ9CaO5/Lv0PA87wCeLBo6OzS2H/oEq9pEEduhYNnV xFEAn1sPEI8bUvvC2HSQKKb9Z5tpMQ4b =vMfh -END PGP SIGNATURE- - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
RE: Question about resetting datasources and changes to the BasicDataSource.close() method
From: Christopher Schultz [mailto:ch...@christopherschultz.net] Subject: Re: Question about resetting datasources and changes to the BasicDataSource.close() method Locking this makes sure that the thread's local copy of whatever-you-are-getting is updated properly and not stale-cached. If you synchronize the setter, it is a very good idea to synchronize the getter too. Also, unless you're absolutely certain that the entity being retrieved is a primitive type, you must synchronize to insure that the structure you're fiddling with is not in an undefined state. - Chuck THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers. - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
RE: Question about resetting datasources and changes to the BasicDataSource.close() method
-Original Message- From: Konstantin Kolinko [mailto:knst.koli...@gmail.com] Sent: Friday, June 01, 2012 11:06 AM By the way: fix bug53254/bug (rev1340160/rev): Add in the ability to purge connections from the pool (fhanik) /fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53254 It will be in Tomcat JDBC pool in 7.0.28. Thank you Konstantin. You inspired me to put my own request in for resizing MaxActive as well. https://issues.apache.org/bugzilla/show_bug.cgi?id=53346 --Brooke - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Question about resetting datasources and changes to the BasicDataSource.close() method
2012/5/30 Hedrick, Brooke - 43 brooke.hedr...@rainhail.com:
(...)
So far, my options point to making changes to the BasicDataSource to provide
a way to set closed=false or make the private method below public with a
change to set closed=false.
/**
* Not used currently
*/
private void restart() { // change to public -bth
try {
close();
closed = false; // new line added here -bth
} catch (SQLException e) {
log(Could not restart DataSource, cause: + e.getMessage());
}
}
Just a note:
If you want this change to be in the code, you have to ask on the
Apache Commons DBCP project.
Tomcat repacks released versions of Commons Pool and Commons DBCP as
they are, only using an automated string replacement to change the
package name.
Best regards,
Konstantin Kolinko
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
RE: Question about resetting datasources and changes to the BasicDataSource.close() method
Chris, We don't have high load applications for the most part so it is pretty common to have a MaxActive set to around 5 or 10. We monitor applications to watch for high-water marks to make sure we keep 10-20% headroom on for free datasource connections. Our release cycle has us releasing many applications on many JVMs every week. From time to time, these applications have issues in them where someone has made a code change that requires 2 connections per request instead of 1. In those cases, we temporarily increase the MaxActive to accommodate the issue introduced. We do this by using JMX as well as updating the server.xml ( in case the JVM is restarted for some reason ). In some cases an application is released that shares a datasource with another application and the MaxActive isn't large enough to accommodate both apps. By using JMX, we are able to increase MaxActive without having to take the JVM down to resolve this. We also have cases where applications are released with connection pool leaks. When this happens, we catch problems because we keep the pools only big enough to service demand. We will see connection pools hit 100% usage and thread pools increase rapidly due to waiting on connections. We do this to prevent the database from being hit overly hard by a single application. We also do not use the removeAbandoned settings today in favor of failing fast. This way the App Server suffers not the DB. Our Tomcat hosts and JVMs greatly outnumber our DB hosts. What do you do today to reset or resize connection pools? Thanks, Brooke Hedrick -Original Message- From: Christopher Schultz [mailto:ch...@christopherschultz.net] Sent: Wednesday, May 30, 2012 7:09 PM To: Tomcat Users List Subject: Re: Question about resetting datasources and changes to the BasicDataSource.close() method -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Brooke, On 5/30/12 3:31 PM, Hedrick, Brooke - 43 wrote: How are others dynamically resetting/resizing their database connection pools when necessary? Quick question about all this: why do you need to do any more dynamic-resizing than dbcp already provides out of the box? I know that dbcp doesn't have a built-in method for trashing all connections and re-filling the pool, but re-sizing is definitely something it can already do (see the configuration property minEvictableIdleTimeMillis). - -chris -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk/GtpEACgkQ9CaO5/Lv0PALnwCgtIQEoDQ8PHSX//pv9NDRg8f+ jQMAn2sxxVMgY4Q5ea7aLvHGkp13tdBO =4tle -END PGP SIGNATURE- - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
RE: Question about resetting datasources and changes to the BasicDataSource.close() method
Thanks Konstantin. I made the change locally, slightly different than what I
suggested. I performed some load tests and called the now public restart()
method. I didn't encounter any exceptions and the pools reset as expected.
This is my work-around though.
How do you reset or resize connection pools?
public synchronized void close() throws SQLException {
if (!restarting) {// bth new check
closed = true;
}
GenericObjectPool oldpool = connectionPool;
connectionPool = null;
dataSource = null;
try {
if (oldpool != null) {
oldpool.close();
}
} catch(SQLException e) {
throw e;
} catch(RuntimeException e) {
throw e;
} catch(Exception e) {
throw new SQLNestedException(Cannot close connection pool, e);
}
}
/**
* Not used currently
*/
protected boolean restarting = false; // bth new
public void restart() {
try {
restarting = true; // bth new
try {
close();
} finally {
restarting = false; // bth new
}
} catch (SQLException e) {
log(Could not restart DataSource, cause: + e.getMessage());
}
}
Thanks,
Brooke Hedrick
Lead Web Administrator
brooke.hedr...@rainhail.com
Tel: 515.559.1322
Cel: 515.314.8953
-Original Message-
From: Konstantin Kolinko [mailto:knst.koli...@gmail.com]
Sent: Thursday, May 31, 2012 8:19 AM
To: Tomcat Users List
Subject: Re: Question about resetting datasources and changes to the
BasicDataSource.close() method
2012/5/30 Hedrick, Brooke - 43 brooke.hedr...@rainhail.com:
(...)
So far, my options point to making changes to the BasicDataSource to provide
a way to set closed=false or make the private method below public with a
change to set closed=false.
/**
* Not used currently
*/
private void restart() { // change to public -bth
try {
close();
closed = false; // new line added here -bth
} catch (SQLException e) {
log(Could not restart DataSource, cause: +
e.getMessage());
}
}
Just a note:
If you want this change to be in the code, you have to ask on the Apache
Commons DBCP project.
Tomcat repacks released versions of Commons Pool and Commons DBCP as they are,
only using an automated string replacement to change the package name.
Best regards,
Konstantin Kolinko
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Question about resetting datasources and changes to the BasicDataSource.close() method
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Brooke, On 5/31/12 11:51 AM, Hedrick, Brooke - 43 wrote: Our release cycle has us releasing many applications on many JVMs every week. From time to time, these applications have issues in them where someone has made a code change that requires 2 connections per request instead of 1. In those cases, we temporarily increase the MaxActive to accommodate the issue introduced. We do this by using JMX as well as updating the server.xml ( in case the JVM is restarted for some reason ). In some cases an application is released that shares a datasource with another application and the MaxActive isn't large enough to accommodate both apps. By using JMX, we are able to increase MaxActive without having to take the JVM down to resolve this. We also have cases where applications are released with connection pool leaks. When this happens, we catch problems because we keep the pools only big enough to service demand. We will see connection pools hit 100% usage and thread pools increase rapidly due to waiting on connections. We do this to prevent the database from being hit overly hard by a single application. We also do not use the removeAbandoned settings today in favor of failing fast. This way the App Server suffers not the DB. Our Tomcat hosts and JVMs greatly outnumber our DB hosts. Gotcha. What do you do today to reset or resize connection pools? We simply use maxActive with log/removeAbandoned. Fortunately, we have a very well-behaved application that neither leaks connections nor requires two connections at once, so we don't have any deadlocks. We run with maxActive=20 and that services our current load requirements with no problems (and no 'abandoned' notifications occur unless some query takes a very long time to execute -- which does happen every few months). We never have to recycle our entire pool for any reason. - -chris -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk/HxX0ACgkQ9CaO5/Lv0PCY+wCdF0++ISNqpl6e2gx4Jp8xXAoI l5QAn1RvRkYlpbXstny7EaChtZI0yqT0 =CCxl -END PGP SIGNATURE- - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Question about resetting datasources and changes to the BasicDataSource.close() method
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Brooke,
On 5/31/12 11:56 AM, Hedrick, Brooke - 43 wrote:
/** * Not used currently */ protected boolean restarting = false;
// bth new
public void restart() { try { restarting = true; // bth new try {
close(); } finally { restarting = false; // bth new } } catch
(SQLException e) { log(Could not restart DataSource, cause: +
e.getMessage()); } }
NB: That code isn't threadsafe.
- -chris
-BEGIN PGP SIGNATURE-
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk/HxfIACgkQ9CaO5/Lv0PB/JwCePtgdjrYr2+Bz1GhjSnZ3SOuY
J4wAmgP/T6YBQUM3BB+nbimJ3FkyEHDu
=dGRE
-END PGP SIGNATURE-
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
RE: Question about resetting datasources and changes to the BasicDataSource.close() method
-Original Message-
From: Christopher Schultz [mailto:ch...@christopherschultz.net]
Sent: Thursday, May 31, 2012 2:27 PM
To: Tomcat Users List
Subject: Re: Question about resetting datasources and changes to the
BasicDataSource.close() method
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Brooke,
On 5/31/12 11:56 AM, Hedrick, Brooke - 43 wrote:
/** * Not used currently */ protected boolean restarting = false; //
bth new
public void restart() { try { restarting = true; // bth new try {
close(); } finally { restarting = false; // bth new } } catch
(SQLException e) { log(Could not restart DataSource, cause: +
e.getMessage()); } }
NB: That code isn't threadsafe.
Chris,
Are you just concerned about the restart() being called by more than 1 thread
since the restarting member variable isn't protected? If so, I was looking for
a short term, simple, fix. Our tools that we use to reset/resize pools manage
not allowing more than 1 JMX call being sent for a single type of operation at
a time. To really fix this, I would rather not put synchronized on the
restart() method because I don't want to have to wrap my brain around
synchronized methods calling synchronized methods - hello potential deadlock.
Plus, the semantics of close already seem odd anyway. What happens if one
thread is running close() and another is asking for a connection via
createDataSource(). The closed variable could be false, createDataSource()
would start to run, get as far as checking to see if the datasource is null.
Next, the close() method nulls out the datasource and createDataSource()
returns the null datasource. The datasource variable itself would need to be
protected/locked.
I am not a Java expert, but this class has some oddness about what is
synchronized. There are getters that are synchronized.
Some examples...
public synchronized boolean getTestWhileIdle() {
return this.testWhileIdle;
}
public synchronized long getMinEvictableIdleTimeMillis() {
return this.minEvictableIdleTimeMillis;
}
public synchronized int getNumTestsPerEvictionRun() {
return this.numTestsPerEvictionRun;
}
--Brooke
Re: Question about resetting datasources and changes to the BasicDataSource.close() method
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Brooke,
On 5/31/12 3:53 PM, Hedrick, Brooke - 43 wrote:
Are you just concerned about the restart() being called by more
than 1 thread since the restarting member variable isn't
protected?
Yes.
If so, I was looking for a short term, simple, fix. Our tools
that we use to reset/resize pools manage not allowing more than 1
JMX call being sent for a single type of operation at a time.
Fair enough. Obviously, any patch to commons-dbcp would have to be
threadsafe, though.
To really fix this, I would rather not put synchronized on the
restart() method because I don't want to have to wrap my brain
around synchronized methods calling synchronized methods - hello
potential deadlock.
Your worries are misplaced: in Java, a synchronized (non-static)
method uses the 'this' as the synchronization monitor and gets
exclusive access to that monitor before continuing (which you knew
already). When calling another synchronized method, the thread already
holds the monitor and so it gets right in -- it's essentially an
un-contended lock at that point so not only is deadlock not a concern,
the operation is very fast.
Plus, the semantics of close already seem odd anyway. What
happens if one thread is running close() and another is asking for
a connection via createDataSource(). The closed variable could be
false, createDataSource() would start to run, get as far as
checking to see if the datasource is null. Next, the close()
method nulls out the datasource and createDataSource() returns the
null datasource. The datasource variable itself would need to be
protected/locked.
Yup: the solution is to just synchronize all the methods. If restart()
is synchronized, it will only operate while other methods are not
actively checking-out or checking-in connections. The big problem with
the concept of a restart is that there could be connections that are
currently checked-out during the call to recycle()... those need to be
handled in some special way -- probably added to a queue that will be
processed later to destroy the connections.
I am not a Java expert, but this class has some oddness about what
is synchronized. There are getters that are synchronized.
Some examples...
public synchronized boolean getTestWhileIdle() { return
this.testWhileIdle; }
public synchronized long getMinEvictableIdleTimeMillis() { return
this.minEvictableIdleTimeMillis; }
public synchronized int getNumTestsPerEvictionRun() { return
this.numTestsPerEvictionRun; }
On the one hand, synchronizing these methods should not be necessary
because Java boolean values are defined to be 32-bit integers which
feature atomic assignment (that is, no thread ever sees only 8-bits of
the 32-bit value being assigned and 24-bits of the old value). On the
other hand, threads are allowed to keep cached copies of certain data
under certain conditions. Using the keyword 'volatile' /should/, in
recent JVMs, make the use of 'synchronized' completely unnecessary,
while older JVMs may even ignore 'volatile' making 'synchronized'
mandatory. Use of 'synchronized' is the safest bet because it
definitely causes a 'memory barrier' to be crossed in any version of
JVM: that means that the thread is required to synchronize (perhaps a
bad choice of words) its cache with main memory which means that all
variables get the latest copies of the real data.
- -chris
-BEGIN PGP SIGNATURE-
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk/H7zgACgkQ9CaO5/Lv0PB/PACfTYSWPc2xxzbQB0FND9HnVMSX
v2kAoKYiQs2Tvc+uQU9RqBEX4HhFtH7d
=Mtj+
-END PGP SIGNATURE-
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Question about resetting datasources and changes to the BasicDataSource.close() method
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Brooke, On 5/30/12 3:31 PM, Hedrick, Brooke - 43 wrote: How are others dynamically resetting/resizing their database connection pools when necessary? Quick question about all this: why do you need to do any more dynamic-resizing than dbcp already provides out of the box? I know that dbcp doesn't have a built-in method for trashing all connections and re-filling the pool, but re-sizing is definitely something it can already do (see the configuration property minEvictableIdleTimeMillis). - -chris -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk/GtpEACgkQ9CaO5/Lv0PALnwCgtIQEoDQ8PHSX//pv9NDRg8f+ jQMAn2sxxVMgY4Q5ea7aLvHGkp13tdBO =4tle -END PGP SIGNATURE- - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
