[PATCH] (DERBY-230) "Schema already exists" when creating a table

[Øystein Grøvlen <Oy...@Sun.COM>]

I have attached a new version of the patch to Derby-230 that should
address the review comments that I received to the first patch.  Could
someone review/commit this?

--
Øystein

Re: [PATCH] (DERBY-230) "Schema already exists" when creating a table

[Øystein Grøvlen <Oy...@Sun.COM>]

>>>>> "A" == Army  <qo...@sbcglobal.net> writes:

    A> Øystein Grøvlen wrote:
    >> I have attached a new version of the patch to Derby-230 that should
    >> address the review comments that I received to the first patch.  Could
    >> someone review/commit this?

    A> I  applied the  patch and  verified  that the  test runs  successfully
    A> against the  server, with  both the JCC  client and the  Derby Network
    A> Client. And  I can see that  you are using ij.startJBMS()  to get all
    A> connections in the test, which is good (per Dan's review comments).


    A> My one  concern is as  follows: I tried  running the new  test withOUT
    A> your fix, and  it actually passed 1  of the 3 times I  ran it (against
    A> embedded mode).  I also  ran it three  times using the  Derby Network
    A> Client and it passed twice.

    A> It  looks like you're  creating 100  threads and  running them  all in
    A> hopes that  the problem will reproduce--which is  _usually_ does. But
    A> it's not guaranteed.

The test is more likely to fail on a multi-CPU computer than on a
single-CPU computer.

    A> This means that if someone  makes a change to break this functionality
    A> in the  future, it's _possible_  (albeit unlikely) that your  new test
    A> will pass  in both embedded and  server modes, and  thus that person's
    A> change will get committed and we'll end up with DERBY-230 reborn.

I agree that it would be nice if the test would always fail.  Since
the bug is pretty minor and has a simple work-around, I think the main
problem with this is not that the bug may be reborn, but that the
reborn bug will probably be detected and create problems for other
developers running the derbyall suite.  In fact, I think such tests
should not be part of a MATS that needs to be run before submitting a
patch, but rather be part of a regression test suite that is run
regularly (e.g. each night).  That way, any rebirth if the bug will
soon be detected without the risk of creating a lot of hazzle for the
developers.

    A> Is there any  way to guarantee that the  test will reproduce? Bumping
    A> the "100" up  to a higher number might help, but  it's still not going
    A> to  give us the  guarantee, so  that's not  really a  solution. Maybe
    A> doing  something  with  autocommit  and  having one  thread  sleep  or
    A> something...I don't  know, I  haven't thought about  it too  much, but
    A> it'd be nice if something like that could be done...

I with experiment a bit with using more threads.  I do not think any
other changes to the test client will increase the probability of
failure much.  In order to get the error one will have to have the
following scenario:

1. Thread 1 checks if the schema exists
2. Thread 2 checks if the schema exists
3. Thread 2 tries to create the schema and succeed
4. Thread 1 tries to create the schema and fails

All this happens within the execution of a single Derby function.
Hence the only way to guarantee that this happens is to make one
thread sleep within this function.  I do not think the seriousness of
this bug and the chance of a rebirth is large enough to clutter the
Derby code with the checking of a debug flag to make threads sleep
between checking for the schema and the creation of the schema.

    A> In short,  I approve of  the fix, but  I'm not sure  the corresponding
    A> test is fully  reliable. Is there any way to  guarantee the test will
    A> catch a failure in this area in the future?

-- 
Øystein

Re: [PATCH] (DERBY-230) "Schema already exists" when creating a table

[Army <qo...@sbcglobal.net>]

Øystein Grøvlen wrote:
> I have attached a new version of the patch to Derby-230 that should
> address the review comments that I received to the first patch.  Could
> someone review/commit this?

I applied the patch and verified that the test runs successfully against the 
server, with both the JCC client and the Derby Network Client.  And I can see 
that you are using ij.startJBMS() to get all connections in the test, which is 
good (per Dan's review comments).

My one concern is as follows: I tried running the new test withOUT your fix, and 
it actually passed 1 of the 3 times I ran it (against embedded mode).  I also 
ran it three times using the Derby Network Client and it passed twice.

It looks like you're creating 100 threads and running them all in hopes that the 
problem will reproduce--which is _usually_ does.  But it's not guaranteed.

This means that if someone makes a change to break this functionality in the 
future, it's _possible_ (albeit unlikely) that your new test will pass in both 
embedded and server modes, and thus that person's change will get committed and 
we'll end up with DERBY-230 reborn.

Is there any way to guarantee that the test will reproduce?  Bumping the "100" 
up to a higher number might help, but it's still not going to give us the 
guarantee, so that's not really a solution.  Maybe doing something with 
autocommit and having one thread sleep or something...I don't know, I haven't 
thought about it too much, but it'd be nice if something like that could be done...

In short, I approve of the fix, but I'm not sure the corresponding test is fully 
reliable.  Is there any way to guarantee the test will catch a failure in this 
area in the future?

Army