[elephant-devel] Re: Updated version of last Postmodern patch bundle

[Leslie P. Polzer]

Dear Alex,

thanks for your extensive comments on the suite.

> i have several notes about concurrent tests:
>
> 1. threaded-idx-access and provoke-deadlock are essentially same -- they
> change slot values of zork instances.

They are designed to provoke different problems, but you're right in
that the thread functions themselves are highly similar.

Let's factor those out, but keep the two tests separate.


>     i do not think any of these differences are essential. having multiple
> slightly different tests hoping they will spot errors is like "voodoo
> programming", i think we should delete provoke-deadlock.

It's not really anything like that. PROVOKE-DEADLOCK is an empirical/stochastical
test, that's right, but that doesn't make it guess-and-miss/-hit.
If there's a problem in deadlock handling, it *will* detect it with a likelihood
very close to 1.


> 2. provoke-deadlock does not wait until threads are finished before it wipes
> classes and closes controller. obviously, this causes weird errors.
>     is it intentianal behaviour?

Not at all. Thanks for pointing out.


> 3. threaded-idx-access joins to all threads it finds. i think it's not OK
> because there might be threads spawned by SLIME etc.

You're right. I don't work with Slime, so I never thought of anyone
having any independent threads except for the master thread.


>     it's better to collect threads created and join to those ones we
> created, i.e.

Yup.


> 4. join-thread seems to be SBCL-specific. can't we find some thread-synch
> primitive that will work on all implementations? (i haven't yet looked into
> this).

For some reason BT doesn't have this function...
We could set up a hash table and let threads set a flag when they're
finished.


> 5. threaded-idx-access is not really an automatic test -- if something gets
> broken, it lands into a debugger.
>     i think that there should be something like handler-case inside each
> thread, and if error happens inside the thread, it should report it to main
> thread, that will re-throw it, or something.

Agreed.


>     also, it's worth checking that operations were correct -- classes are
> accessible via index, slot values are correct etc.

The tests are not really aiming for correctness, they were designed to
trigger exceptional situations. We already have the serialization
correctness tests. But adding some checking along these lines would
be good, of course.


> now, about test runs. indeed, it yields lots of _different_ deadlocks in
> isolation mode "read commited".

What do you mean by "different"?


> conclusions are clear to me: use serializable isolation mode FTW.
> it's not worth trying to fix working in "read committed" mode, because there
> is no logical grounds for it to be working fine when data can be changed at
> any time and it not consistent within a single transaction.

It's consistent enough for my purposes, so I didn't bother.


> i think it's better to completely ban "read committed" mode, but i can make
> this configurable for people with masochistic intentions :).

That's fine by me.


> as i have ideas how to "improve" concurrent test suite, probably it would be
> better if i'll just take over the suite and implement them. however, i'm
> open about other ideas about future of this test suite :).

Go ahead and take it, you seem to be more adamant about it than me :)
As you have noticed, it's quite hackish ATM.


> also i have a question unrelated to test suite: it says me that
> thread-alive-p used in reap-orphaned-connections in db-postmodern is not
> defined. where should this function come from, newer version of bordeax
> threads (perhaps i have an outdated package)?

I have 0.2.2 here. The API docs have it, too, albeit in a section accompanied
by this note:

"The following functions may be provided for debugging purposes, but are not advised to
be called from normal user code."

Grepping through the source shows that it's defined for every supported
implementation except MCL and Corman CL.

  Leslie