From mboxrd@z Thu Jan 1 00:00:00 1970 From: Danny Milosavljevic Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Date: Mon, 27 Aug 2018 16:23:53 +0200 Message-ID: <20180827162353.1bdeef85@scratchpost.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> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; boundary="Sig_/bUux25L=SetgvRg92wn2lLF"; protocol="application/pgp-signature" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:44271) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fuIew-0001gL-QJ for bug-guix@gnu.org; Mon, 27 Aug 2018 10:38:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fuIRy-0007Nv-Fm for bug-guix@gnu.org; Mon, 27 Aug 2018 10:25:08 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:57602) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fuIRy-0007Nr-AY for bug-guix@gnu.org; Mon, 27 Aug 2018 10:25:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fuIRy-0006Gs-3b for bug-guix@gnu.org; Mon, 27 Aug 2018 10:25:02 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <87a7p8aqy6.fsf@lassieur.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: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: 32234@debbugs.gnu.org --Sig_/bUux25L=SetgvRg92wn2lLF Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Cl=C3=A9ment, I've read through the patch and it seems to handle the case I mean fine bec= ause you support an arbitrary number of queries per db critical section - so I a= gree that this patchset is fine. Keep in mind this is only fine if the critical section is held over an enti= re http request handler and not only over a single database query (as far as I can = 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 data= base connection > > in order for the web interface to be isolated while it's querying. =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 b= in/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 q= uerying one > > page, half of the things have different values than you requested - whi= ch is really > > weird. > > > > For example right now you could query for a row with status=3D42 and ge= t back data with > > status=3D43 (because it has been changed in the mean time). =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 foll= ow - finding the problem by debugging the Javascript frontend (wrongly) was much more di= fficult): 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 ex= tra 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 fine. 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. --Sig_/bUux25L=SetgvRg92wn2lLF Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAluECXkACgkQ5xo1VCww uqWURQf/ZYdyJFUDuWF/5BVOShbztA7ConM30pKkAHOeErII0BFfksQinP3a8PoD 97wby+jlzoCbVyHARdxchPinM1ln1D9Dty8OWWokSTMNPiZ+7LiNjM+1UNbhoTmJ LaKN2UhAnxokOHATr4wdVb95ldErueBkaEZxySsS7HE/xJtqq5IzNVm2N2TVpvDH RNsEzVmx0LOg0vfqr3YgAxX+9Nj6BxfERjwWg0g5vdDqkNxN2aV8kNzvJ70xucSa AAxDYMWUJ1lVNzNAjhBFP1A87ETNuqd+a0RiiPCELO3KRzhEMxbZv+IXlxb+/iWw IKMt6Za7fWf5ReJ0DAZpggYZgrWTFw== =JW8p -----END PGP SIGNATURE----- --Sig_/bUux25L=SetgvRg92wn2lLF--