CLSQL-Fluid
Scott L. Burson
I'm just starting to try Weblocks for a project. And of course,
downloading it with Quicklisp I immediately run into the need for
CLSQL-Fluid -- which I can't download right at the moment, apparently,
because b9.com's Git server is down. Presumably that will get fixed,
but I'm also wondering (partly prompted by the comments on
http://www.cliki.net/clsql-fluid ) whether CLSQL-Fluid is really the
right thing. Wouldn't something like this work just as well? If not,
why not?
(defmethod accept-connections :around ((acceptor weblocks-acceptor))
(clsql:with-database (clsql:*default-database* *connection-
spec* :pool t)
(call-next-method)))
-- Scott
Leslie P. Polzer
good to see you around here!
> but I'm also wondering (partly prompted by the comments onhttp://www.cliki.net/clsql-fluid) whether CLSQL-Fluid is really the
> why not?
>
> (defmethod accept-connections :around ((acceptor weblocks-acceptor))
> (clsql:with-database (clsql:*default-database* *connection-
> spec* :pool t)
> (call-next-method)))
and think it's the right thing to do (provided we abstract it properly
so it works with multiple stores).
But we need to test and think a bit more about it. It'd be great if
you could try this approach in parallel for CLSQL.
Leslie
Scott Burson
> We've discussed this in the context of the upcoming Postmodern store
> and think it's the right thing to do (provided we abstract it properly
> so it works with multiple stores).
>
> But we need to test and think a bit more about it. It'd be great if
> you could try this approach in parallel for CLSQL.
Okay, I took a shot at it. I quickly discovered the complication
posed by multiple stores. But I think I found a reasonably elegant
solution. It cheats just a tiny bit, by calling the internal accessor
CLSQL-SYS::CONNECTION-SPEC, but that's all. It seems to work, though
all I've done with it so far is run the demo.
The patch is below. Basically there's a new generic function
THREAD-PREPARE-STORE by which each store gets to do setup and teardown
for each thread. PROCESS-CONNECTION (not ACCEPT-CONNECTIONS, as I
originally thought) is made to call this.
I added an (OPEN-STORES) call in START-WEBLOCKS, and a (CLOSE-STORES)
call in STOP-WEBLOCKS. The former is necessary, because the stores
have to be opened before the server threads get created in order for
THREAD-PREPARE-STORE to do its thing. The latter isn't strictly
necessary, but I noticed that STOP-WEBLOCKS wasn't closing stores as
its doc string promises.
To convince yourself it's working, trace CLSQL:CONNECT and
CLSQL:DISCONNECT. (You probably know, but I didn't: if you're using
slime-repl, trace output from background threads appears in
*inferior-lisp*.)
-- Scott
diff -r 5659d7f4ca80 src/acceptor.lisp
--- a/src/acceptor.lisp Wed Dec 15 12:56:55 2010 +0100
+++ b/src/acceptor.lisp Wed Dec 22 12:04:33 2010 -0800
@@ -20,3 +20,7 @@
(let ((*print-readably* nil))
(call-next-method)))
+(defmethod process-connection :around ((acceptor weblocks-acceptor) socket)
+ (with-stores-thread-prepared
+ (call-next-method)))
+
diff -r 5659d7f4ca80 src/server.lisp
--- a/src/server.lisp Wed Dec 15 12:56:55 2010 +0100
+++ b/src/server.lisp Wed Dec 22 12:04:33 2010 -0800
@@ -56,6 +56,7 @@
(if debug
(enable-global-debugging)
(disable-global-debugging))
+ (open-stores)
(when (null *weblocks-server*)
(values
(start (setf *weblocks-server*
@@ -75,6 +76,7 @@
(reset-sessions)
(when *weblocks-server*
(stop *weblocks-server*))
+ (close-stores)
(setf *weblocks-server* nil)))
diff -r 5659d7f4ca80 src/store/clsql/clsql.lisp
--- a/src/store/clsql/clsql.lisp Wed Dec 15 12:56:55 2010 +0100
+++ b/src/store/clsql/clsql.lisp Wed Dec 22 12:04:33 2010 -0800
@@ -10,13 +10,18 @@
(export '(order-by-expression range-to-offset range-to-limit))
+(defmethod thread-prepare-store ((store database) thunk)
+ (with-database (*default-database* (clsql-sys::connection-spec store)
+ :database-type (database-type store)
+ :pool t :if-exists :new)
+ (funcall thunk)))
+
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; Initialization/finalization ;;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(defmethod open-store ((store-type (eql :clsql)) &rest args)
(setf *default-caching* nil)
- (setf *default-store* (apply #'make-instance 'fluid-database
- :connection-spec args)))
+ (setf *default-store* (apply #'connect args)))
(defmethod close-store ((store database))
(when (eq *default-store* store)
diff -r 5659d7f4ca80 src/store/clsql/weblocks-clsql.asd
--- a/src/store/clsql/weblocks-clsql.asd Wed Dec 15 12:56:55 2010 +0100
+++ b/src/store/clsql/weblocks-clsql.asd Wed Dec 22 12:04:33 2010 -0800
@@ -11,6 +11,6 @@
:author "Slava Akhmechet"
:licence "LLGPL"
:description "A weblocks backend for clsql."
- :depends-on (:closer-mop :metatilities :clsql :clsql-fluid :weblocks)
+ :depends-on (:closer-mop :metatilities :clsql :weblocks)
:components ((:file "clsql")))
diff -r 5659d7f4ca80 src/store/store-api.lisp
--- a/src/store/store-api.lisp Wed Dec 15 12:56:55 2010 +0100
+++ b/src/store/store-api.lisp Wed Dec 22 12:04:33 2010 -0800
@@ -1,7 +1,7 @@
(in-package :weblocks)
-(export '(open-store close-store clean-store *default-store*
+(export '(open-store close-store clean-store thread-prepare-store
*default-store*
begin-transaction commit-transaction rollback-transaction
dynamic-transaction use-dynamic-transaction-p
persist-object delete-persistent-object
@@ -26,6 +26,10 @@
should erase data, but not necessarily any schema information (like
tables, etc.)"))
+(defgeneric thread-prepare-store (store thunk)
+ (:documentation "Some stores need per-thread setup/teardown. This
function does
+ the setup, calls THUNK (a function of no arguments), and does the
teardown."))
+
(defvar *default-store* nil
"The default store to which objects are persisted. Bound while a
webapp is handling a request to the value of its
diff -r 5659d7f4ca80 src/store/store-utils.lisp
--- a/src/store/store-utils.lisp Wed Dec 15 12:56:55 2010 +0100
+++ b/src/store/store-utils.lisp Wed Dec 22 12:04:33 2010 -0800
@@ -158,3 +158,21 @@
(dolist (obj objects)
(apply #'persist-object store obj keys)))
+;;; Default method.
+(defmethod thread-prepare-store ((store t) thunk)
+ (funcall thunk))
+
+(defun thread-prepare-stores (thunk)
+ (labels ((rec (store-names)
+ (if store-names
+ (if (symbol-value (car store-names))
+ (thread-prepare-store (symbol-value (car store-names))
+ #'(lambda ()
+ (rec (cdr store-names))))
+ (rec (cdr store-names)))
+ (funcall thunk))))
+ (rec *store-names*)))
+
+(defmacro with-stores-thread-prepared (&body body)
+ `(thread-prepare-stores #'(lambda () . ,body)))
+
Stephen Compall
> why not?
http://groups.google.com/group/weblocks/msg/ebd9125eaf328e77
Has CLSQL been changed so that standard-db-objects no longer hold onto
their creating connections? If not, with-database will not be
sufficient.
Take care when testing -- this didn't show up in casual testing. It's
the kind of thing that works great until you go to production.
--
Stephen Compall
Greetings from sunny Appleton!
Scott Burson
<stephen...@gmail.com> wrote:
>
> On Dec 20, 9:07 pm, "Scott L. Burson" <g...@zeta-soft.com> wrote:
>> Presumably that will get fixed,
>> but I'm also wondering (partly prompted by the comments onhttp://www.cliki.net/clsql-fluid) whether CLSQL-Fluid is really the
>> right thing. Wouldn't something like this work just as well? If not,
>> why not?
>
> I doubt that with-database is actually sufficient.
>
> http://groups.google.com/group/weblocks/msg/ebd9125eaf328e77
Hmm, okay. It seems to be sufficient to allow me to begin developing
my app, which solves my immediate problem.
I do think that it's unfortunate that one can't get a working Weblocks
on CLSQL via Quicklisp (alone). It looks like the ways this could be
dealt with are:
(0) Live with the restrictions/bugs imposed by WITH-DATABASE.
(1) Persuade Kevin Rosenberg that this problem needs to be solved in
CLSQL, either by merging CLSQL-Fluid or perhaps some other way.
(2) Fork CLSQL.
I understand that (0) is undesirable, and (2) is not so great either.
Has much effort been put towards (1)? Whoever modified the CLiki page
(Kevin?) thinks that the primary issue is thread safety -- which was
my initial impression as well. It turns out that's just one problem;
the other one, as you say, is lazy initialization of join slots. This
problem is not Weblocks-specific, correct?
-- Scott
Leslie P. Polzer
> Hmm, okay. It seems to be sufficient to allow me to begin developing
> my app, which solves my immediate problem.
along. I like your solution because it ties in neatly with what we've
planned for the new Postmodern store.
The problem is in the CLSQL code, and we should strive to fix it in
their code and submit a patch against the official repository or at
worst devise our own hack for it if that isn't feasible for some
reason.
Leslie
Scott Burson
> On Wed, Dec 22, 2010 at 12:33 PM, Stephen Compall
> <stephen...@gmail.com> wrote:
>>
>> I doubt that with-database is actually sufficient.
>>
> (Kevin?) thinks that the primary issue is thread safety -- which was
> my initial impression as well. It turns out that's just one problem;
> the other one, as you say, is lazy initialization of join slots. This
> problem is not Weblocks-specific, correct?
Let me try again. The problem _is_ thread-safety, but WITH-DATABASE
is not sufficient, because a db-object could be created in one thread
and then have a lazy join slot accessed in a different thread. In
that case, CLSQL will use the connection from the first thread, and
that connection could be in use by some other thread (particularly
given that threads are being created and destroyed and the connections
are pooled; the thread that created the object might be long gone and
its connection in use by a new thread).
There, does that describe the problem well? If so, then I would think
one solution would simply be for CLSQL to lock-protect the
connections. This would lead to undesirable levels of contention if a
single connection were in use for the entire app, but given the
changes I'm suggesting, threads would normally use their own
connections; only in the case of lazy join slot initialization would
contention be possible, and therefore the amount of contention would
be minimal.
If that's right, it might also be an easier solution to get Kevin to
accept, given that he has already demonstrated no eagerness to merge
CLSQL-Fluid.
-- Scott
Scott L. Burson
<leslie...@gmx.net> wrote:
> On Dec 23, 8:34 am, Scott Burson <g...@zeta-soft.com> wrote:
>
>> Hmm, okay. It seems to be sufficient to allow me to begin developing
>> my app, which solves my immediate problem.
>
> We can solve any problems that come up with your approach as you go
> along. I like your solution because it ties in neatly with what we've
> planned for the new Postmodern store.
So the Postmodern backend has a similar problem? I'm wondering if I
should be trying to use Postmodern instead of CLSQL -- but my very
superficial impression, when I looked at both of them, was that CLSQL
looked more mature and fully-featured. Postmodern is an option,
though, because I'm using Postgres anyway. The CLSQL query syntax
does look a bit weird, but I don't know if Postmodern's is really that
much of an improvement.
> The problem is in the CLSQL code, and we should strive to fix it in
> their code and submit a patch against the official repository or at
> worst devise our own hack for it if that isn't feasible for some
> reason.
Well, of course, Stephen already tried submitting a patch once.
That's why I'm wondering how much effort was put into persuading Kevin
to accept it.
-- Scott
Brit Butler
> So the postmodern backend has a similar problem?
The postmodern code is indeed new and not ready for real use until
this issue is resolved. I'm hoping to take a look at it over the next
few days. The plan is to add store-thread-setup, store-thread-teardown
and use-thread-hooks-p generics to the Store API which get called in
handle-client-request. I'll post to the list when I have a first pass
at the patch. Hopefully that will be before Monday.
Cheers,
Brit Butler
On Dec 23, 2:20 pm, "Scott L. Burson" <Sc...@ergy.com> wrote:
> On Wed, Dec 22, 2010 at 11:43 PM, Leslie P. Polzer
>
Scott Burson
> Scott,
>
>> So the postmodern backend has a similar problem?
>
> The postmodern code is indeed new and not ready for real use until
> this issue is resolved. I'm hoping to take a look at it over the next
> few days. The plan is to add store-thread-setup, store-thread-teardown
> and use-thread-hooks-p generics to the Store API which get called in
> handle-client-request. I'll post to the list when I have a first pass
> at the patch. Hopefully that will be before Monday.
Ah, okay. Sounds like we're duplicating some effort -- you might want
to take a look at the patch I included in a previous message in this
thread.
-- Scott
>
> On Dec 23, 2:20 pm, "Scott L. Burson" <Sc...@ergy.com> wrote:
>> On Wed, Dec 22, 2010 at 11:43 PM, Leslie P. Polzer
>>
>> <leslie.pol...@gmx.net> wrote:
>> > On Dec 23, 8:34 am, Scott Burson <g...@zeta-soft.com> wrote:
>>
>> >> Hmm, okay. It seems to be sufficient to allow me to begin developing
>> >> my app, which solves my immediate problem.
>>
>> > We can solve any problems that come up with your approach as you go
>> > along. I like your solution because it ties in neatly with what we've
>> > planned for the new Postmodern store.
>>
>> So the Postmodern backend has a similar problem? I'm wondering if I
>> should be trying to use Postmodern instead of CLSQL -- but my very
>> superficial impression, when I looked at both of them, was that CLSQL
>> looked more mature and fully-featured. Postmodern is an option,
>> though, because I'm using Postgres anyway. The CLSQL query syntax
>> does look a bit weird, but I don't know if Postmodern's is really that
>> much of an improvement.
>>
>> > The problem is in the CLSQL code, and we should strive to fix it in
>> > their code and submit a patch against the official repository or at
>> > worst devise our own hack for it if that isn't feasible for some
>> > reason.
>>
>> Well, of course, Stephen already tried submitting a patch once.
>> That's why I'm wondering how much effort was put into persuading Kevin
>> to accept it.
>>
>> -- Scott
>
> --
> You received this message because you are subscribed to the Google Groups "weblocks" group.
> To post to this group, send email to webl...@googlegroups.com.
> To unsubscribe from this group, send email to weblocks+u...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/weblocks?hl=en.
>
>
Andrea Russo
Stephen Compall <stephen...@gmail.com> writes:
> I doubt that with-database is actually sufficient.
>
> http://groups.google.com/group/weblocks/msg/ebd9125eaf328e77
>
> Has CLSQL been changed so that standard-db-objects no longer hold onto
> their creating connections? If not, with-database will not be
> sufficient.
It seems that with CLSQL6 (now available from http://git.b9.com/clsql.git)
is now possible to have standard-db-objects use other connections than
their creating one.
The technique is described here:
http://russ.unwashedmeme.com/blog/?p=218
So, now, `with-database' and a customized `choose-database-for-instance'
method, should be sufficient.
Time to remove my lock in handle-client-request ;-)
Bye,
Andrea.
--
Software vuol dire conoscenza! Per questo dev'essere un bene comune!
Usa software libero e contribisci a cambiare il mondo:
http://www.softwarelibero.it
http://www.fsf.org
http://www.gnu.org
Scott Burson
> On Tue, Dec 21, 2010 at 4:19 AM, Leslie P. Polzer <leslie...@gmx.net> wrote:
>> We've discussed this in the context of the upcoming Postmodern store
>> and think it's the right thing to do (provided we abstract it properly
>> so it works with multiple stores).
>>
>> But we need to test and think a bit more about it. It'd be great if
>> you could try this approach in parallel for CLSQL.
>
>
> The patch is below. Basically there's a new generic function
> THREAD-PREPARE-STORE
I don't know if anybody ever tried to use this patch, but I hope not,
because it doesn't work. I mean, it appears to work at first, but in
the end it turns out not to.
The problem is that it opens new connections and binds
*DEFAULT-DATABASE* on a request basis, but that's too fine-grained.
Widgets hold on to view objects across requests. There's also the
CLASS-STORE slot of DATA-EDITOR, whose lifetime is longer than a
request.
I now understand that CLSQL-Fluid represents the right solution
strategy. I considered the approach of associating database
connections with sessions, which I think would also basically work;
the problem, though, is that a busy server could have many thousands
of active sessions (particularly if one cranks up Hunchentoot's
default 30-minute session timeout to something more user-friendly,
like 72 hours), and I don't know if it's wise to have thousands of
open connections to a database, even if most of them aren't doing
anything at any given moment.
There's precedent, by the way, for something like CLSQL-Fluid. It's
the same idea as synonym streams: delegation of operations on a
per-thread basis.
I haven't looked at what you've done for the Postmodern store, but if
it's the same idea as what I did here, you might need to reconsider.
-- Scott