On 10/01/2010, Phil Steitz <phil.ste...@gmail.com> wrote:
> sebb wrote:
>  > On 10/01/2010, Phil Steitz <phil.ste...@gmail.com> wrote:
>  >> sebb wrote:
>  >>  > Thanks.
>  >>  >
>  >>  > Looks like I did not complete the fixes properly when I added the
>  >>  > loopOnce parameter to PoolTest.
>  >>
>  >>  I think I found (and fixed) another problem.  See r897720.
>  >
>  > The wait in question is purely to allow the threads to start, so I
>  > think it should not depend on the value of maxWait (or indeed
>  > holdTime). Originally it was set to 10L * holdTime, which meant that
>  > it did always not work well for holdTime = 1.
>
>
> OK, then I must be missing something.  Since stop can interrupt run,
>  it would seem to me that entering the stop loop immediately after
>  this wait is going to stop all of the threads, giving them all just
>  whatever the wait is to complete.

In the case where the loop only operates once, so long as the wait
time is long enough to allow all the threads to start then it won't
affect further processing of the PoolTest threads. However if it is
much longer than maxWait or holdTime, the test will take longer than
necessary.

For the multi-loop case, the wait time must again be enough to allow
the threads to start. This guarantees that the threads will run at
least once. The overall run-time of the test (and the number of loops)
is controlled by the wait time, because the holdTime is very short in
comparison.

>  That, I was assuming, is why the
>  1.2.2 version waited 10 * holdTime, which would not really have been
>  quite right - better a function of maxWait.

The 1.2.2 version also handled completion differently - it did not
wait for the threads to complete, and it did not have a loopOnce
option. Provided that maxWait expired before holdTime, then at least
one thread would fail, and this would stop all the threads. But this
was not always happening, so I introduced the loopOnce parameter.

>
>  Phil
>
>
>  >
>  >>  >
>  >>  > It was quite tricky following the Continuum build output, as the date
>  >>  > was 2 days behind, and the mail for the failed runs is not always
>  >>  > accurate - if the "Exit code" is 127, then most of the email contents
>  >>  > is inaccurate.
>  >>  >
>  >>  > I have raised http://jira.codehaus.org/browse/CONTINUUM-2428 for the
>  >>  > misleading info.
>  >>  >
>  >>  > On 10/01/2010, Phil Steitz <phil.ste...@gmail.com> wrote:
>  >>  >> From the debugging added to some previously failed builds, I saw
>  >>  >>  loop = 2 for some threads.  Threads should not be looping.  Second
>  >>  >>  loop by a thread that succeeded the first time that throws will not
>  >>  >>  change success state - so it looks like a success -> not enough
>  >>  >>  failures.
>  >>  >>
>  >>  >>  Phil
>  >>  >>
>  >>  >>
>  >>  >>  pste...@apache.org wrote:
>  >>  >>  > Author: psteitz
>  >>  >>  > Date: Sun Jan 10 18:21:03 2010
>  >>  >>  > New Revision: 897678
>  >>  >>  >
>  >>  >>  > URL: http://svn.apache.org/viewvc?rev=897678&view=rev
>  >>  >>  > Log:
>  >>  >>  > Eliminated unintended looping in mutipleThreads test.
>  >>  >>  >
>  >>  >>  > Modified:
>  >>  >>  >     
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestConnectionPool.java
>  >>  >>  >     
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestPerUserPoolDataSource.java
>  >>  >>  >     
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java
>  >>  >>  >
>  >>  >>  > Modified: 
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestConnectionPool.java
>  >>  >>  > URL: 
> http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestConnectionPool.java?rev=897678&r1=897677&r2=897678&view=diff
>  >>  >>  > 
> ==============================================================================
>  >>  >>  > --- 
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestConnectionPool.java
>  (original)
>  >>  >>  > +++ 
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestConnectionPool.java
>  Sun Jan 10 18:21:03 2010
>  >>  >>  > @@ -683,7 +683,21 @@
>  >>  >>  >          }
>  >>  >>  >      }
>  >>  >>  >
>  >>  >>  > -    protected void multipleThreads(final int holdTime, final 
> boolean expectError, long maxWait)
>  >>  >>  > +    /**
>  >>  >>  > +     * Launches a group of 2 * getMaxActive() threads, each of 
> which will attempt to obtain a connection
>  >>  >>  > +     * from the pool, hold it for <holdTime> ms, and then return 
> it to the pool.  If <loopOnce> is false,
>  >>  >>  > +     * threads will continue this process indefinitely.  If 
> <expectingError> is true, exactly 1/2 of the
>  >>  >>  > +     * threads are expected to either throw exceptions or fail to 
> complete. If <expectingError> is false,
>  >>  >>  > +     * all threads are expected to complete successfully.
>  >>  >>  > +     *
>  >>  >>  > +     * @param holdTime time in ms that a thread holds a 
> connection before returning it to the pool
>  >>  >>  > +     * @param expectError whether or not an error is expected
>  >>  >>  > +     * @param loopOnce whether threads should complete the borrow 
> - hold - return cycle only once, or loop indefinitely
>  >>  >>  > +     * @param maxWait passed in by client - has no impact on the 
> test itself, but does get reported
>  >>  >>  > +     *
>  >>  >>  > +     * @throws Exception
>  >>  >>  > +     */
>  >>  >>  > +    protected void multipleThreads(final int holdTime, final 
> boolean expectError, final boolean loopOnce, final long maxWait)
>  >>  >>  >              throws Exception {
>  >>  >>  >                  long startTime = timeStamp();
>  >>  >>  >                  final PoolTest[] pts = new PoolTest[2 * 
> getMaxActive()];
>  >>  >>  > @@ -696,8 +710,7 @@
>  >>  >>  >                      }
>  >>  >>  >                  };
>  >>  >>  >                  for (int i = 0; i < pts.length; i++) {
>  >>  >>  > -                    // If we are expecting an error, don't allow 
> successful threads to loop
>  >>  >>  > -                    (pts[i] = new PoolTest(threadGroup, holdTime, 
> expectError)).start();
>  >>  >>  > +                    (pts[i] = new PoolTest(threadGroup, holdTime, 
> expectError, loopOnce)).start();
>  >>  >>  >                  }
>  >>  >>  >
>  >>  >>  >                  Thread.sleep(100L); // Wait for long enough to 
> allow threads to start
>  >>  >>  >
>  >>  >>  > Modified: 
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestPerUserPoolDataSource.java
>  >>  >>  > URL: 
> http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestPerUserPoolDataSource.java?rev=897678&r1=897677&r2=897678&view=diff
>  >>  >>  > 
> ==============================================================================
>  >>  >>  > --- 
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestPerUserPoolDataSource.java
>  (original)
>  >>  >>  > +++ 
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestPerUserPoolDataSource.java
>  Sun Jan 10 18:21:03 2010
>  >>  >>  > @@ -378,11 +378,11 @@
>  >>  >>  >          final int defaultMaxWait = 430;
>  >>  >>  >          ((PerUserPoolDataSource) 
> ds).setDefaultMaxWait(defaultMaxWait);
>  >>  >>  >          ((PerUserPoolDataSource) ds).setPerUserMaxWait("foo",new 
> Integer(defaultMaxWait));
>  >>  >>  > -        multipleThreads(1, false, defaultMaxWait);
>  >>  >>  > +        multipleThreads(1, false, false, defaultMaxWait);
>  >>  >>  >      }
>  >>  >>  >
>  >>  >>  >      public void testMultipleThreads2() throws Exception {
>  >>  >>  > -        multipleThreads(2 * (int)(getMaxWait()), true, 
> getMaxWait());
>  >>  >>  > +        multipleThreads(2 * (int)(getMaxWait()), true, false, 
> getMaxWait());
>  >>  >>  >      }
>  >>  >>  >
>  >>  >>  >      public void testTransactionIsolationBehavior() throws 
> Exception {
>  >>  >>  >
>  >>  >>  > Modified: 
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java
>  >>  >>  > URL: 
> http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java?rev=897678&r1=897677&r2=897678&view=diff
>  >>  >>  > 
> ==============================================================================
>  >>  >>  > --- 
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java
>  (original)
>  >>  >>  > +++ 
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java
>  Sun Jan 10 18:21:03 2010
>  >>  >>  > @@ -368,11 +368,11 @@
>  >>  >>  >          // some JVMs, e.g. Windows.
>  >>  >>  >          final int defaultMaxWait = 430;
>  >>  >>  >          ((SharedPoolDataSource) ds).setMaxWait(defaultMaxWait);
>  >>  >>  > -        multipleThreads(1, false, defaultMaxWait);
>  >>  >>  > +        multipleThreads(1, false, false, defaultMaxWait);
>  >>  >>  >      }
>  >>  >>  >
>  >>  >>  >      public void testMultipleThreads2() throws Exception {
>  >>  >>  > -        multipleThreads(2 * (int)(getMaxWait()), true, 
> getMaxWait());
>  >>  >>  > +        multipleThreads(2 * (int)(getMaxWait()), true, false, 
> getMaxWait());
>  >>  >>  >      }
>  >>  >>  >
>  >>  >>  >      public void testTransactionIsolationBehavior() throws 
> Exception {
>  >>  >>  >
>  >>  >>  >
>  >>  >>
>  >>  >>
>  >>  >>
>  >>  >> ---------------------------------------------------------------------
>  >>  >>  To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>  >>  >>  For additional commands, e-mail: dev-h...@commons.apache.org
>  >>  >>
>  >>  >>
>  >>  >
>  >>  > ---------------------------------------------------------------------
>  >>  > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>  >>  > For additional commands, e-mail: dev-h...@commons.apache.org
>  >>  >
>  >>
>  >>
>  >>  ---------------------------------------------------------------------
>  >>  To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>  >>  For additional commands, e-mail: dev-h...@commons.apache.org
>  >>
>  >>
>  >
>  > ---------------------------------------------------------------------
>  > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>  > For additional commands, e-mail: dev-h...@commons.apache.org
>  >
>
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>  For additional commands, e-mail: dev-h...@commons.apache.org
>
>

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

Reply via email to