Re: update-records-from-instance and thread safety

Nathan Bird <[email protected]> Tue, 24 Apr 2012 11:15:41 -0400
Newsgroups gmane.lisp.clsql.general
Message-ID <[email protected]>
On 04/13/2012 08:53 AM, JTK wrote:
>
>
> Please forgive any ignorance or misunderstandings in what follows, and 
> any presumption in offering
> solutions to problems I might not fully understand ...
>
> First, I'll describe my understanding of what happens with updates ...
>
> When a STANDARD-DB-OBJECT is read from a database, its VIEW-DATABASE
> slot contains the database.
>
> This allows UPDATE-RECORDS-FROM-INSTANCE to work.  When this method
> sees the VIEW-DATABASE slot set, it does an SQL update; otherwise it 
> uses an SQL insert.
>
> You cannot override the VIEW-DATABASE slot using the :DATABASE keyword in
> UPDATE-RECORDS-FROM-INSTANCE.  The VIEW-DATABASE takes precedence.
>
> However, this presents a problem for threads.   If you read an object 
> in one thread
> and pass it to another and update it, then you could have two threads 
> using the same
> connection, which is very bad, perhaps fatal.
>
> And if you use pooled connections, you can read an object in thread A, 
> return the database to the pool,
> where it gets picked up by thread B.  Then you update the object in 
> thread A, and threads
> A and B fight over the same connection.
>
> So I was motivated to try several workarounds -
>
> 1) not use pools - this used up connection resources and gave mysql 
> errors.  I think there is some
>      latency in closing a connection (port number) and having it be 
> usable again.
>
> 2) wrote a safe version of UPDATE-RECORDS-FROM-INSTANCE that switched out
>      VIEW-DATABASE  for fresh-from-the-pool database object in an 
> UNWIND-PROTECT, and then calls
>      the regular UPDATE-RECORDS-FROM-INSTANCE.  This is kludgy, but it 
> works, if you use it everywhere.
>
>
> Other better solutions might be
>
> 3) put a lock inside every DATABASE object.  This would work, but when 
> a database connection gets returned
>   to the pool, you'd have mysterious locking up of thread A (the 
> thread that owns an object being updated with
>   VIEW-DATABASE in it) by thread B (who innocently acquired the 
> database connection from the pool).
>
> 4) my suggestion - don't put the database in the VIEW-DATABASE slot, 
> but instead put
>      the CONNECTION-SPEC, DATABASE-TYPE, and maybe ENCODING of the 
> database.
>      This would allow UPDATE-RECORDS-FROM-INSTANCE
>      to use WITH-DATABASE to draw a database from the 
> already-thread-safe database pool.
>      Problem: what about :IF-EXISTS?
>
>
> 5) Other solutions like http://russ.unwashedmeme.com/blog/?p=218  and 
> http://common-lisp.net/project/clsql-fluid/ .
>     I guess what I don't like about the first is that it defines a 
> special thread-safe database class, when it would be nice
>     to solve this problem everywhere.   I'm not sure about the status 
> of the last one.  At any rate, it seems that the thread
>     safety problem ought to be addressed, because it seems likely to 
> hit anyone who uses clsql with threads.
>
> Finally, apologies in advance if I just barged in and spouted ignorance.
>
> J.Klein
>


We talked about this problem several years ago and the compromise 
solution we came to was the choose-database-for-instance function. The 
issue is that some people were/are using clsql in more of a desktop app 
type setting where a long running connection tied to the object makes a 
bit of sense. We didn't want to break people's existing code so we 
introduced this function that maintained the current behavior but 
allowed for easy overriding as explained in Russ' blog post you linked 
above.

That's the solution we've been running (with the odbc backend) for a 
while now, just make sure you have the generic function loaded early on.

Your suggestion number 4 sounds like another good path; if you want to 
make a patch based on it we can give it a try. Some other random pieces 
you should be aware of if you want to try: clsql has some query-result 
caching (i've always had this turned off), transactions, ... I guess 
that's all.

Cheers,
Nathan Bird

_______________________________________________
CLSQL mailing list
[email protected]
http://lists.b9.com/cgi-bin/mailman/listinfo/clsql