From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Date: Mon, 27 Aug 2018 17:05:03 +0200 Message-ID: <874lffc0kg.fsf@lassieur.org> References: <20180806192736.1747-1-clement@lassieur.org> <20180806192736.1747-2-clement@lassieur.org> <87zhxih2no.fsf@gnu.org> <87lg8tb4rz.fsf@lassieur.org> <20180827144120.13c76371@scratchpost.org> <87a7p8aqy6.fsf@lassieur.org> <20180827162353.1bdeef85@scratchpost.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:60950) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fuJDj-0001ot-1f for bug-guix@gnu.org; Mon, 27 Aug 2018 11:14:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fuJ5e-0004RI-7z for bug-guix@gnu.org; Mon, 27 Aug 2018 11:06:04 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:57621) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fuJ5e-0004Q8-0j for bug-guix@gnu.org; Mon, 27 Aug 2018 11:06:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fuJ5d-0007If-Md for bug-guix@gnu.org; Mon, 27 Aug 2018 11:06:01 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-reply-to: <20180827162353.1bdeef85@scratchpost.org> List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Danny Milosavljevic Cc: 32234@debbugs.gnu.org Thank you for the explanation Danny. Indeed I didn't fix what you described. That could be done easily by wrapping the handler with WITH-DB-CRITICAL-SECTION. I'm not sure about the consequences in terms of performance, given that this will send a huge function to a channel, and that all the work will be done in the same thread. If you think it's worth it, don't hesitate to send a patch. Cl=C3=A9ment Danny Milosavljevic writes: > Hi Cl=C3=A9ment, > > I've read through the patch and it seems to handle the case I mean fine b= ecause > you support an arbitrary number of queries per db critical section - so I= agree > that this patchset is fine. > > Keep in mind this is only fine if the critical section is held over an en= tire http > request handler and not only over a single database query (as far as I ca= n see > the former is the case in the patch - OK).=20 > > Much longer explanation follows: > > On Mon, 27 Aug 2018 15:18:09 +0200 > Cl=C3=A9ment Lassieur wrote: > >> Danny Milosavljevic writes: >>=20 >> > Hi Cl=C3=A9ment, >> > >> > in the future I plan on making the actual bin/evaluate use another dat= abase connection >> > in order for the web interface to be isolated while it's querying.=20= =20 >>=20 >> I don't understand... bin/evaluate doesn't query the database at all. I >> don't know why it would need to. > > Yeah, it has moved. Sorry. > > But I mean the part that changes the values in the database (on behalf of= bin/evaluate). > So now it's the procedure "evaluate" in src/cuirass/base.scm . > >> > Otherwise - as it is now in master - it can happen that while you are = querying one >> > page, half of the things have different values than you requested - wh= ich is really >> > weird. >> > >> > For example right now you could query for a row with status=3D42 and g= et back data with >> > status=3D43 (because it has been changed in the mean time).=20=20 >>=20 >> Could you please show an example that I can reproduce? I don't >> understand what you mean. > > Right now something like this happens (simplified to make it easier to fo= llow - finding > the problem by debugging the Javascript frontend (wrongly) was much more = difficult): > > Connection 1: > > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > > Connection 2: > > ... Wait some time until the user sends a request... > Query: SELECT x FROM a > Result: Nondeterministic number > Query: SELECT x FROM a > Result: Nondeterministic possibly different number (WTF!!!!!) > > This is especially bad if you request extra data from other tables in an = extra > query and the join condition suddenly doesn't match (and thus the row isn= 't > returned!). > > > Better would be if it acted like this: > > Connection 1: > > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > Statement: UPDATE a SET x =3D x + 1 > > Connection 2: > > ... Wait some time until the user sends a request... > Statement: BEGIN TRANSACTION > Statement: SET TRANSACTION ISOLATION LEVEL SERIALIZABLE > Query: SELECT x FROM a > Result: Some number > Query: SELECT x FROM a > Result: The same number > ... wait however long you want > Query: SELECT x FROM a > Result: The same number > Statement: ROLLBACK TRANSACTION or COMMIT TRANSACTION > > loop > > Statement: BEGIN TRANSACTION > Statement: SET TRANSACTION ISOLATION LEVEL SERIALIZABLE > Query: SELECT x FROM a > Result: Some possibly different number xxx > Query: SELECT x FROM a > Result: The same number xxx as in the previous query > Query: SELECT x FROM a > Result: The same number xxx as in the previous query > ... > >> With that patch, database queries are serialized, which means that if >> query1, query2 and query3 are sent in that order, they will be executed >> in that order, one after the other. > > It depends on what exactly that means. As long as it means that the > entire HTTP request handler is ONE query that is ordered such, that's fin= e. > > Otherwise not. > > If there are more complicated multiple queries done by the web interface > on behalf of the user because of one HTTP request, we have to make sure > that those queries execute without any interleaving by some writer. > > As a stopgap, this database query serializer should let the user batch > the queries/statements and run each batch in its own transaction. > I think that would be quite okay as a solution, actually, as long as > there are no other shadow clients of the database. > >> Currently, there is only one connection that is shared by the writers >> and readers. Having one 'read connection' and one 'write connection' >> would be possible and make sense if both of them live in a separate >> thread and use the SQLite multithreading feature so that writing and >> reading proceed concurrently. Is that what you mean? > > No.