db-jdo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Erik Bengtson <e...@jpox.org>
Subject Re: TCK tests : QueryCancel
Date Sun, 14 Mar 2010 21:03:32 GMT
If you provide a query parameter and lock the parameter object in your
thread, you might be able to have the other thread waiting you to
unlock, so you can execute the cancel.

On Sun, Mar 14, 2010 at 7:50 PM, Michael Bouschen <mbo.tech@spree.de> wrote:
> Hi Andy, hi Craig,
>
> I'm surprised about the query execution time of 60 ms to 2 ms. Andy, your
> hardware seems to be much faster than mine :-).
>
> I agree we should change the test such that the query takes longer to
> execute. I propose to create more test instances, then the join (as part of
> the query) becomes more expensive.
>
> Attached you find a patch with the following changes of the test class:
> - it creates 5000 PCPoint and PCPoint2 instances (instead of 1000)
> - uses a barrier to synchronize the threads
> - compiles the query before staring the thread
>
> Regards Michael
>
>> Hi Andy,
>>
>> On Mar 13, 2010, at 3:29 AM, Andy Jefferson wrote:
>>
>>
>>>
>>> Hi Michael,
>>>
>>>
>>>>
>>>> I added a Thread.sleep(1000) to the main thread in order to give the
>>>> other thread a chance to start the query before the main thread  cancels
>>>> it. Could you please give it a try?
>>>>
>>>
>>> Giving something a second to get started seems excessive, I know  Derby
>>> is slow
>>> but ... ;-)
>>> I tried with a value more like 80 or 100 (millis) and managed **on
>>>  occasions**
>>> to get a JDOQueryInterruptedException (and pass the test). Other  times
>>> the
>>> cancel still got in first. I also added a query.compile() before
>>>  starting the
>>> threads hence less for the query execute to do before starting, but
>>>  still
>>> intermittent. The intermittency will also be affected by the machine
>>>  being
>>> used, hence the number is arbitrary. The query execution time (in the
>>> datastore) is anywhere between 60ms and 2ms on my machine.
>>>
>>
>> This test will always be somewhat sensitive to timing, but we should  be
>> able to minimize the timing effects.
>>
>> The query itself should be long enough to allow the cancel to have an
>>  effect. The query execution time of 60 ms to 2 ms. [sic] doesn't sound
>>  long enough. Maybe we need to make the query more complex. But still,  as
>> machines get faster the query execution times will get smaller.
>>
>> The other thing we discussed earlier was the possibility of needing a
>>  barrier so we could guarantee that the two threads were synchronized  at
>> some point. The main thread calling start doesn't quite guarantee
>>  synchronization, while a barrier of size two should do it. Still,  assuming
>> that the query is compiled before the barrier, and the main  thread yields
>> to the query thread (sleep should do this just fine) we  should be able to
>> get the query to start and the cancel to interrupt it.
>>
>>>
>>> Depends what the cancel is allowing cancel over; to be consistent  with
>>> the
>>> timeout I assume it is cancel the datastore operation (as opposed to  the
>>> whole
>>> query execute process ... compile, generate PreparedStatement,  populate
>>> parameter values, execute in the datastore, return results), and so
>>> DataNucleus only starts the execution in a separate thread and  maintains
>>> the
>>> handle on that thread for cancellation purposes. If this execution
>>>  thread is
>>> either not yet created, or is now finished then the test will fail,
>>>  always.
>>>
>>
>> Another good high level discussion. Since we have the datastore read
>>  timeout, the purpose of the cancel is to stop query execution before  the
>> timeout expires. Since we assume that the main thread and the  query thread
>> have ways of communicating as long as the query thread is  not stuck in a
>> datastore wait, we are only concerned about getting the  query thread
>> unstuck.
>>
>> So if the query has already returned results, the purpose for cancel
>>  doesn't exist. There still may be a timing issue in real life where  the
>> main thread sees that the query thread is stuck, but by the time  it gets
>> around to canceling the query, the query thread is no longer  stuck. So it
>> doesn't really matter whether the cancel worked or not.  That's why I agree
>> that a reasonable behavior is having the cancel be  a no-op in case it can't
>> find anything to do.
>>
>> But we still want a test case. Do we need the test case to perform  some
>> timing tests to see how long queries take and adjust the wait  time
>> accordingly? Do we want to try different values and require that  at least
>> one value succeed? Start with a value of 1 ms and raise by  increments until
>> the cancel works?
>>
>>>
>>> Shouldn't the test also take into account if the query does actually
>>>  return
>>> the results before the cancel can be called ?
>>>
>>
>> This could be part of the heuristics as noted above. Much better if we
>>  can figure out a good barrier/sleep strategy that at least works for  now.
>>
>> Craig
>>
>>>
>>> Regards
>>> --
>>> Andy
>>> DataNucleus (http://www.datanucleus.org)
>>>
>>
>> Craig L Russell
>> Architect, Oracle
>> http://db.apache.org/jdo
>> 408 276-5638 mailto:Craig.Russell@oracle.com
>> P.S. A good JDO? O, Gasp!
>>
>>
>
>
> --
> *Michael Bouschen*
> *Prokurist*
>
> akquinet tech@spree GmbH
> Bülowstr. 66, D-10783 Berlin
>
> Fon:   +49 30 235 520-33
> Fax:   +49 30 217 520-12
> Email: michael.bouschen@akquinet.de
> Url:    www.akquinet.de <http://www.akquinet.de>
>
> akquinet tech@spree GmbH, Berlin
> Geschäftsführung: Martin Weber, Prof. Dr. Christian Roth
> Amtsgericht Berlin-Charlottenburg HRB 86780 B
> USt.-Id. Nr.: DE 225 964 680
>
> Index: src/java/org/apache/jdo/tck/query/api/QueryCancel.java
> ===================================================================
> --- src/java/org/apache/jdo/tck/query/api/QueryCancel.java      (Revision
> 922445)
> +++ src/java/org/apache/jdo/tck/query/api/QueryCancel.java
>  (Arbeitskopie)
> @@ -17,6 +17,8 @@
>
>  package org.apache.jdo.tck.query.api;
>
> +import java.util.concurrent.CyclicBarrier;
> +
>  import junit.framework.AssertionFailedError;
>
>  import javax.jdo.JDOFatalException;
> @@ -50,8 +52,11 @@
>  public class QueryCancel extends QueryTest {
>
>     /** Time for the main thread to sleep after starting a parallel thread.
> */
> -    private static int MAIN_SLEEP_MILLIS = 1000;
> +    private static int MAIN_SLEEP_MILLIS = 20;
>
> +    /** Number of instances to be created. */
> +    private static int NO_OF_INSTANCES = 5000;
> +
>     /** */
>     private static final String ASSERTION_FAILED =
>         "Assertion A14.6.1-8 (QueryCancel) failed: ";
> @@ -77,18 +82,24 @@
>     /** */
>     public void testCancel() throws Exception {
>         PersistenceManager pm = getPM();
> +        // Test query
>         Query query = pm.newQuery(SSJDOQL);
> +        query.compile();
>
>         // Thread executing the query
> +        CyclicBarrier barrier = new CyclicBarrier(2);
>         ThreadExceptionHandler group = new ThreadExceptionHandler();
> -        QueryExecutor runnable = new QueryExecutor(pm, query);
> +        QueryExecutor runnable = new QueryExecutor(pm, query, barrier);
>         Thread t = new Thread(group, runnable, "Query Executor");
>         t.start();
>
> -        // Wait for a second such that the other thread can execute the
> query
> -        Thread.sleep(MAIN_SLEEP_MILLIS);
> +        try {
> +            // Wait for the other thread
> +            barrier.await();
>
> -        try {
> +            // Wait a couple of millis such that the other thread can start
> query execution
> +            Thread.sleep(MAIN_SLEEP_MILLIS);
> +
>             // cancel query
>             query.cancel(t);
>             if (!isQueryCancelSupported()) {
> @@ -119,19 +130,25 @@
>     /** */
>     public void testCancelAll() throws Exception {
>         PersistenceManager pm = getPM();
> +        // Test query
>         Query query = pm.newQuery(SSJDOQL);
> +        query.compile();
>
>         // Thread executing the query
> +        CyclicBarrier barrier = new CyclicBarrier(2);
>         ThreadExceptionHandler group = new ThreadExceptionHandler();
> -        QueryExecutor runnable = new QueryExecutor(pm, query);
> +        QueryExecutor runnable = new QueryExecutor(pm, query, barrier);
>         Thread t = new Thread(group, runnable, "Query Executor");
>         t.start();
>
> -        // Wait for a second such that the other thread can execute the
> query
> -        Thread.sleep(MAIN_SLEEP_MILLIS);
> -
>         try {
>             // cancel query
> +            // Wait for the other thread
> +            barrier.await();
> +
> +            // Wait a couple of millis such that the other thread can start
> query execution
> +            Thread.sleep(MAIN_SLEEP_MILLIS);
> +
>             query.cancelAll();
>             if (!isQueryCancelSupported()) {
>                 fail(ASSERTION_FAILED,
> @@ -162,17 +179,23 @@
>     class QueryExecutor implements Runnable {
>
>         PersistenceManager pm;
> +        CyclicBarrier barrier;
>         Query query;
>
> -        QueryExecutor(PersistenceManager pm, Query query) {
> +        QueryExecutor(PersistenceManager pm, Query query, CyclicBarrier
> barrier) {
>             this.pm = pm;
>             this.query = query;
> +            this.barrier = barrier;
>         }
>
>         public void run() {
>             Transaction tx = pm.currentTransaction();
>             try {
>                 tx.begin();
> +
> +                // wait for the other thread
> +                barrier.await();
> +
>                 Object result = query.execute();
>                 tx.commit();
>                 tx = null;
> @@ -189,6 +212,9 @@
>                          "if query canceling is not supported.");
>                 }
>             }
> +            catch (Exception ex) {
> +                throw new RuntimeException(ex);
> +            }
>             finally {
>                 if ((tx != null) && tx.isActive())
>                     tx.rollback();
> @@ -208,11 +234,11 @@
>         Transaction tx = pm.currentTransaction();
>         try {
>             tx.begin();
> -            for (int i = 0; i < 1000; i++) {
> +            for (int i = 0; i < NO_OF_INSTANCES; i++) {
>                 PCPoint obj = new PCPoint(i, i);
>                 pm.makePersistent(obj);
>             }
> -            for (int i = 0; i < 1000; i++) {
> +            for (int i = 0; i < NO_OF_INSTANCES; i++) {
>                 PCPoint2 obj = new PCPoint2(i, i);
>                 pm.makePersistent(obj);
>             }
>
>

Mime
View raw message