From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: Cuirass: The SQLite built in busy handler might block the Fibers scheduler Resent-From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Sat, 21 Jul 2018 09:59:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: 32234@debbugs.gnu.org X-Debbugs-Original-To: bug-guix@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.153216710110394 (code B ref -1); Sat, 21 Jul 2018 09:59:01 +0000 Received: (at submit) by debbugs.gnu.org; 21 Jul 2018 09:58:21 +0000 Received: from localhost ([127.0.0.1]:50219 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fgoeX-0002hW-EH for submit@debbugs.gnu.org; Sat, 21 Jul 2018 05:58:21 -0400 Received: from eggs.gnu.org ([208.118.235.92]:43839) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fgoeV-0002hJ-8F for submit@debbugs.gnu.org; Sat, 21 Jul 2018 05:58:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fgoeP-0001f4-3o for submit@debbugs.gnu.org; Sat, 21 Jul 2018 05:58:09 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50 autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:56937) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fgoeP-0001ex-0t for submit@debbugs.gnu.org; Sat, 21 Jul 2018 05:58:09 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34810) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fgoeN-0001if-Hr for bug-guix@gnu.org; Sat, 21 Jul 2018 05:58:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fgoeK-0001e6-HP for bug-guix@gnu.org; Sat, 21 Jul 2018 05:58:07 -0400 Received: from mail.lassieur.org ([83.152.10.219]:60320) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fgoeK-0001dS-57 for bug-guix@gnu.org; Sat, 21 Jul 2018 05:58:04 -0400 Received: from rodion (88.191.118.83 [88.191.118.83]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id 94315a1b (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO) for ; Sat, 21 Jul 2018 09:57:58 +0000 (UTC) User-agent: mu4e 1.0; emacs 26.1 From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Date: Sat, 21 Jul 2018 11:57:57 +0200 Message-ID: <87k1ponc62.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -4.1 (----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -5.1 (-----) Hi, I'm trying to understand why Berlin web API times out[1]. I think the SQlite built in busy handler may block the Fibers scheduler. We use "PRAGMA busy_timeout =3D 30000;", which is an alternative to calling sqlite3_busy_timeout(), whose description[2] is: This routine sets a busy handler that sleeps for a specified amount of time when a table is locked. The handler will sleep multiple times until at least "ms" milliseconds of sleeping have accumulated. After at least "ms" milliseconds of sleeping, the handler returns 0 which causes sqlite3_step() to return SQLITE_BUSY. To me this sounds like non-cooperative and non-resumable code. A solution would be to set a custom handler through sqlite3_busy_handler[3] that would be Fibers compatible, i.e. it would let the scheduler schedule other fibers instead of just sleeping, using Fibers 'sleep' procedure[4]. WDYT? Cl=C3=A9ment [1]: https://bugs.gnu.org/32233. [2]: https://www.sqlite.org/c3ref/busy_timeout.html [3]: https://www.sqlite.org/c3ref/busy_handler.html [4]: https://github.com/wingo/fibers/wiki/Manual#user-content-index-sleep From debbugs-submit-bounces@debbugs.gnu.org Sat Jul 21 08:40:35 2018 Received: (at control) by debbugs.gnu.org; 21 Jul 2018 12:40:35 +0000 Received: from localhost ([127.0.0.1]:50293 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fgrBa-00028G-VG for submit@debbugs.gnu.org; Sat, 21 Jul 2018 08:40:35 -0400 Received: from mail.lassieur.org ([83.152.10.219]:50050) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fgrBZ-000285-9M for control@debbugs.gnu.org; Sat, 21 Jul 2018 08:40:33 -0400 Received: from rodion (37.170.115.37 [37.170.115.37]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id 4ae42ea9 (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO) for ; Sat, 21 Jul 2018 12:40:31 +0000 (UTC) Date: Sat, 21 Jul 2018 14:40:30 +0200 Message-Id: <87in58n4n5.fsf@lassieur.org> To: control@debbugs.gnu.org From: clement@lassieur.org (=?utf-8?Q?Cl=C3=A9ment?= Lassieur) Subject: control message for bug #32233 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: control X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) block 32233 by 32234 From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: Cuirass: The SQLite built in busy handler might block the Fibers scheduler Resent-From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 23 Jul 2018 09:19:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.15323375388167 (code B ref 32234); Mon, 23 Jul 2018 09:19:02 +0000 Received: (at 32234) by debbugs.gnu.org; 23 Jul 2018 09:18:58 +0000 Received: from localhost ([127.0.0.1]:53075 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fhWzZ-00027f-M0 for submit@debbugs.gnu.org; Mon, 23 Jul 2018 05:18:57 -0400 Received: from eggs.gnu.org ([208.118.235.92]:35775) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fhWzY-00027S-KG for 32234@debbugs.gnu.org; Mon, 23 Jul 2018 05:18:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fhWzP-0007Pq-8o for 32234@debbugs.gnu.org; Mon, 23 Jul 2018 05:18:51 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-0.0 required=5.0 tests=BAYES_20 autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:33926) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fhWzP-0007Pm-3p; Mon, 23 Jul 2018 05:18:47 -0400 Received: from [193.50.110.227] (port=34578 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1fhWzO-00018m-5K; Mon, 23 Jul 2018 05:18:46 -0400 From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) References: <87k1ponc62.fsf@lassieur.org> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 5 Thermidor an 226 de la =?UTF-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Mon, 23 Jul 2018 11:18:43 +0200 In-Reply-To: <87k1ponc62.fsf@lassieur.org> ("=?UTF-8?Q?Cl=C3=A9ment?= Lassieur"'s message of "Sat, 21 Jul 2018 11:57:57 +0200") Message-ID: <87bmaytimk.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -5.0 (-----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -6.0 (------) Hi, Cl=C3=A9ment Lassieur skribis: > I'm trying to understand why Berlin web API times out[1]. Just to be clear, it does block on things involving non-trivial database queries: --8<---------------cut here---------------start------------->8--- ludo@berlin ~$ time wget -q -O - http://localhost:8081/specifications [{"name" : "core-updates-core-updates","load-path-inputs" : ["core-updates"= ],"package-path-inputs" : [],"proc-input" : "core-updates","proc-file" : "b= uild-aux/cuirass/gnu-system.scm","proc" : "cuirass-jobs","proc-args" : {"su= bset" : "core","systems" : ["x86_64-linux", "i686-linux"]},"inputs" : [{"na= me" : "core-updates","url" : "https://git.savannah.gnu.org/git/guix.git","l= oad-path" : ".","branch" : "core-updates","tag" : false,"commit" : false,"n= o-compile?" : true}]}, {"name" : "staging-staging","load-path-inputs" : ["s= taging"],"package-path-inputs" : [],"proc-input" : "staging","proc-file" : = "build-aux/cuirass/gnu-system.scm","proc" : "cuirass-jobs","proc-args" : {"= systems" : ["x86_64-linux", "i686-linux"]},"inputs" : [{"name" : "staging",= "url" : "https://git.savannah.gnu.org/git/guix.git","load-path" : ".","bran= ch" : "staging","tag" : false,"commit" : false,"no-compile?" : true}]}, {"n= ame" : "guix-modular-master","load-path-inputs" : ["guix-modular"],"package= -path-inputs" : [],"proc-input" : "guix-modular","proc-file" : "build-aux/c= uirass/guix-modular.scm","proc" : "cuirass-jobs","proc-args" : {"systems" := ["x86_64-linux", "i686-linux"]},"inputs" : [{"name" : "guix-modular","url"= : "https://git.savannah.gnu.org/git/guix.git","load-path" : ".","branch" := "master","tag" : false,"commit" : false,"no-compile?" : true}]}, {"name" := "guix-master","load-path-inputs" : ["guix"],"package-path-inputs" : [],"pr= oc-input" : "guix","proc-file" : "build-aux/cuirass/gnu-system.scm","proc" = : "cuirass-jobs","proc-args" : {"subset" : "all","systems" : ["x86_64-linux= ", "i686-linux"]},"inputs" : [{"name" : "guix","url" : "https://git.savanna= h.gnu.org/git/guix.git","load-path" : ".","branch" : "master","tag" : false= ,"commit" : false,"no-compile?" : true}]}] real 0m0.007s user 0m0.004s sys 0m0.000s ludo@berlin ~$ time wget -q -O - http://localhost:8081/api/latestbuilds?nr= =3D2 [{"id" : 4484197,"jobset" : "core-updates-core-updates","job" : "ghc-alex-3= .2.3.i686-linux","timestamp" : 1532336715,"starttime" : 1532336542,"stoptim= e" : 1532336715,"derivation" : "/gnu/store/b072sfmw44ww9dsm1ahpqafjk05zsl04= -ghc-alex-3.2.3.drv","buildoutputs" : {"out" : {"path" : "/gnu/store/qk79zl= 1gcg6lh4k2nrp72x8iywy317d2-ghc-alex-3.2.3"}},"system" : "i686-linux","nixna= me" : "ghc-alex-3.2.3","buildstatus" : 0,"busy" : 0,"priority" : 0,"finishe= d" : 1,"buildproducts" : null,"releasename" : null,"buildinputs_builds" : n= ull}, {"id" : 4799156,"jobset" : "core-updates-core-updates","job" : "ghc-a= lex-3.2.3.i686-linux","timestamp" : 1532336715,"starttime" : 1532336542,"st= optime" : 1532336715,"derivation" : "/gnu/store/b072sfmw44ww9dsm1ahpqafjk05= zsl04-ghc-alex-3.2.3.drv","buildoutputs" : {"out" : {"path" : "/gnu/store/q= k79zl1gcg6lh4k2nrp72x8iywy317d2-ghc-alex-3.2.3"}},"system" : "i686-linux","= nixname" : "ghc-alex-3.2.3","buildstatus" : 0,"busy" : 0,"priority" : 0,"fi= nished" : 1,"buildproducts" : null,"releasename" : null,"buildinputs_builds= " : null}] real 0m28.033s user 0m0.004s sys 0m0.000s --8<---------------cut here---------------end--------------->8--- I=E2=80=99m not sure it=E2=80=99s due to locking though; we may lack an ind= ex or two, =E2=80=9Cexplain query plan=E2=80=9D should help. > I think the SQlite built in busy handler may block the Fibers scheduler. > We use "PRAGMA busy_timeout =3D 30000;", which is an alternative to > calling sqlite3_busy_timeout(), whose description[2] is: > > This routine sets a busy handler that sleeps for a specified amount > of time when a table is locked. The handler will sleep multiple > times until at least "ms" milliseconds of sleeping have > accumulated. After at least "ms" milliseconds of sleeping, the > handler returns 0 which causes sqlite3_step() to return SQLITE_BUSY. > > To me this sounds like non-cooperative and non-resumable code. Indeed! > A solution would be to set a custom handler through > sqlite3_busy_handler[3] that would be Fibers compatible, i.e. it would > let the scheduler schedule other fibers instead of just sleeping, using > Fibers 'sleep' procedure[4]. AIUI the handler is called from C, and thus it=E2=80=99s a non-resumable continuation, so this wouldn=E2=80=99t work. Perhaps instead we need to set the timeout to a small value and handle SQLITE_BUSY at the call site in our code. We could define a macro that automatically retries upon SQLITE_BUSY. Thoughts? Ludo=E2=80=99. From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: Cuirass: The SQLite built in busy handler might block the Fibers scheduler Resent-From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 23 Jul 2018 13:43:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153235334815538 (code B ref 32234); Mon, 23 Jul 2018 13:43:01 +0000 Received: (at 32234) by debbugs.gnu.org; 23 Jul 2018 13:42:28 +0000 Received: from localhost ([127.0.0.1]:53227 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fhb6Z-00042Y-Rg for submit@debbugs.gnu.org; Mon, 23 Jul 2018 09:42:28 -0400 Received: from mail.lassieur.org ([83.152.10.219]:50278) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fhb6Y-00042P-Aj for 32234@debbugs.gnu.org; Mon, 23 Jul 2018 09:42:26 -0400 Received: from newt (smtp.parrot.biz [62.23.167.188]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id ce607a20 (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO); Mon, 23 Jul 2018 13:42:24 +0000 (UTC) References: <87k1ponc62.fsf@lassieur.org> <87bmaytimk.fsf@gnu.org> User-agent: mu4e 1.0; emacs 26.1 From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <87bmaytimk.fsf@gnu.org> Date: Mon, 23 Jul 2018 15:42:24 +0200 Message-ID: <87tvoqt6f3.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Ludovic Court=C3=A8s writes: >> I think the SQlite built in busy handler may block the Fibers scheduler. >> We use "PRAGMA busy_timeout =3D 30000;", which is an alternative to >> calling sqlite3_busy_timeout(), whose description[2] is: >> >> This routine sets a busy handler that sleeps for a specified amount >> of time when a table is locked. The handler will sleep multiple >> times until at least "ms" milliseconds of sleeping have >> accumulated. After at least "ms" milliseconds of sleeping, the >> handler returns 0 which causes sqlite3_step() to return SQLITE_BUSY. >> >> To me this sounds like non-cooperative and non-resumable code. > > Indeed! > >> A solution would be to set a custom handler through >> sqlite3_busy_handler[3] that would be Fibers compatible, i.e. it would >> let the scheduler schedule other fibers instead of just sleeping, using >> Fibers 'sleep' procedure[4]. > > AIUI the handler is called from C, and thus it=E2=80=99s a non-resumable > continuation, so this wouldn=E2=80=99t work. Oh, I see. > Perhaps instead we need to set the timeout to a small value and handle > SQLITE_BUSY at the call site in our code. We could define a macro that > automatically retries upon SQLITE_BUSY. That would limit the issue to the first timeout span: for that short time the scheduler would be blocked. I think a timeout of 0 would be better. Another solution would be to serialize all the database accesses as we do already with the url handler, and stop using the SQLITE multithreading features. It would probably make the code simpler because we would use the same paradigm everywhere, and we would avoid looping until SQLITE isn't busy at each request. WDYT? Cl=C3=A9ment From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: Cuirass: The SQLite built in busy handler might block the Fibers scheduler Resent-From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 23 Jul 2018 14:59:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153235788223084 (code B ref 32234); Mon, 23 Jul 2018 14:59:02 +0000 Received: (at 32234) by debbugs.gnu.org; 23 Jul 2018 14:58:02 +0000 Received: from localhost ([127.0.0.1]:54232 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fhcHi-000604-24 for submit@debbugs.gnu.org; Mon, 23 Jul 2018 10:58:02 -0400 Received: from eggs.gnu.org ([208.118.235.92]:56775) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fhcHf-0005zl-UR for 32234@debbugs.gnu.org; Mon, 23 Jul 2018 10:58:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fhcHX-0000fc-HH for 32234@debbugs.gnu.org; Mon, 23 Jul 2018 10:57:54 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-0.0 required=5.0 tests=BAYES_20 autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:39731) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fhcHX-0000fV-C9; Mon, 23 Jul 2018 10:57:51 -0400 Received: from [193.50.110.227] (port=34898 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1fhcHW-00043t-TV; Mon, 23 Jul 2018 10:57:51 -0400 From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) References: <87k1ponc62.fsf@lassieur.org> <87bmaytimk.fsf@gnu.org> <87tvoqt6f3.fsf@lassieur.org> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 5 Thermidor an 226 de la =?UTF-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Mon, 23 Jul 2018 16:57:49 +0200 In-Reply-To: <87tvoqt6f3.fsf@lassieur.org> ("=?UTF-8?Q?Cl=C3=A9ment?= Lassieur"'s message of "Mon, 23 Jul 2018 15:42:24 +0200") Message-ID: <87y3e2ngnm.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -5.0 (-----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -6.0 (------) Cl=C3=A9ment Lassieur skribis: > Ludovic Court=C3=A8s writes: [...] >> Perhaps instead we need to set the timeout to a small value and handle >> SQLITE_BUSY at the call site in our code. We could define a macro that >> automatically retries upon SQLITE_BUSY. > > That would limit the issue to the first timeout span: for that short > time the scheduler would be blocked. I think a timeout of 0 would be > better. Yes, 0 is an acceptable =E2=80=9Csmall value.=E2=80=9D ;-) Perhaps 100ms = would be acceptable if the situation is rare enough, dunno. > Another solution would be to serialize all the database accesses as we > do already with the url handler, and stop using the SQLITE > multithreading features. It would probably make the code simpler > because we would use the same paradigm everywhere, and we would avoid > looping until SQLITE isn't busy at each request. In essence we=E2=80=99d introduce a =E2=80=9Cdatabase server=E2=80=9D runni= ng as a fiber, and everyone would talk to that server. I considered doing that before but then though sqlite would probably be able to do better than this, but I don=E2=80=99t know. What=E2=80=99s a bit annoying with switching to a database server model is = that we=E2=80=99d need to adapt every call site. Thoughts? Ludo=E2=80=99. From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: Cuirass: The SQLite built in busy handler might block the Fibers scheduler Resent-From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 23 Jul 2018 16:19:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153236269930516 (code B ref 32234); Mon, 23 Jul 2018 16:19:02 +0000 Received: (at 32234) by debbugs.gnu.org; 23 Jul 2018 16:18:19 +0000 Received: from localhost ([127.0.0.1]:54266 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fhdXP-0007w7-Df for submit@debbugs.gnu.org; Mon, 23 Jul 2018 12:18:19 -0400 Received: from mail.lassieur.org ([83.152.10.219]:50290) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fhdXN-0007vx-6l for 32234@debbugs.gnu.org; Mon, 23 Jul 2018 12:18:17 -0400 Received: from newt (smtp.parrot.biz [62.23.167.188]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id 9cf26673 (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO); Mon, 23 Jul 2018 16:18:15 +0000 (UTC) References: <87k1ponc62.fsf@lassieur.org> <87bmaytimk.fsf@gnu.org> <87tvoqt6f3.fsf@lassieur.org> <87y3e2ngnm.fsf@gnu.org> User-agent: mu4e 1.0; emacs 26.1 From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <87y3e2ngnm.fsf@gnu.org> Date: Mon, 23 Jul 2018 18:18:14 +0200 Message-ID: <87r2jusz7d.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Ludovic Court=C3=A8s writes: > Cl=C3=A9ment Lassieur skribis: > >> Ludovic Court=C3=A8s writes: > > [...] > >>> Perhaps instead we need to set the timeout to a small value and handle >>> SQLITE_BUSY at the call site in our code. We could define a macro that >>> automatically retries upon SQLITE_BUSY. >> >> That would limit the issue to the first timeout span: for that short >> time the scheduler would be blocked. I think a timeout of 0 would be >> better. > > Yes, 0 is an acceptable =E2=80=9Csmall value.=E2=80=9D ;-) Perhaps 100m= s would be > acceptable if the situation is rare enough, dunno. > >> Another solution would be to serialize all the database accesses as we >> do already with the url handler, and stop using the SQLITE >> multithreading features. It would probably make the code simpler >> because we would use the same paradigm everywhere, and we would avoid >> looping until SQLITE isn't busy at each request. > > In essence we=E2=80=99d introduce a =E2=80=9Cdatabase server=E2=80=9D run= ning as a fiber, and > everyone would talk to that server. Why not as a thread? There would be only one thread dedicated to SQLITE (thus adding a constant overhead), and that would prevent the Scheduler from being bloqued by long SQLite queries. > I considered doing that before but then though sqlite would probably be > able to do better than this, but I don=E2=80=99t know. And it's hard to test... > What=E2=80=99s a bit annoying with switching to a database server model i= s that > we=E2=80=99d need to adapt every call site. > > Thoughts? I'm a tiny bit in favor of switching to a database server model because it's more consistent overall (we already use it for the url handler) and the scheduler wouldn't be bloqued by long SQLite queries. It's annoying to adapt every call site, but it would be done once and for all. :-) So... as you wish! Cl=C3=A9ment From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: Cuirass: The SQLite built in busy handler might block the Fibers scheduler Resent-From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 23 Jul 2018 20:12:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153237667819115 (code B ref 32234); Mon, 23 Jul 2018 20:12:02 +0000 Received: (at 32234) by debbugs.gnu.org; 23 Jul 2018 20:11:18 +0000 Received: from localhost ([127.0.0.1]:54370 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fhhAs-0004yF-Dv for submit@debbugs.gnu.org; Mon, 23 Jul 2018 16:11:18 -0400 Received: from eggs.gnu.org ([208.118.235.92]:47346) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fhhAp-0004y1-6D for 32234@debbugs.gnu.org; Mon, 23 Jul 2018 16:11:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fhhAj-0000V4-0f for 32234@debbugs.gnu.org; Mon, 23 Jul 2018 16:11:10 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-0.0 required=5.0 tests=BAYES_40 autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:44327) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fhhAi-0000Uq-RG; Mon, 23 Jul 2018 16:11:08 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=50338 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1fhhAi-0006kh-FA; Mon, 23 Jul 2018 16:11:08 -0400 From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) References: <87k1ponc62.fsf@lassieur.org> <87bmaytimk.fsf@gnu.org> <87tvoqt6f3.fsf@lassieur.org> <87y3e2ngnm.fsf@gnu.org> <87r2jusz7d.fsf@lassieur.org> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 5 Thermidor an 226 de la =?UTF-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Mon, 23 Jul 2018 22:11:07 +0200 In-Reply-To: <87r2jusz7d.fsf@lassieur.org> ("=?UTF-8?Q?Cl=C3=A9ment?= Lassieur"'s message of "Mon, 23 Jul 2018 18:18:14 +0200") Message-ID: <8736w9ogpw.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -5.0 (-----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -6.0 (------) Cl=C3=A9ment Lassieur skribis: > Ludovic Court=C3=A8s writes: > >> Cl=C3=A9ment Lassieur skribis: >> >>> Ludovic Court=C3=A8s writes: >> >> [...] >> >>>> Perhaps instead we need to set the timeout to a small value and handle >>>> SQLITE_BUSY at the call site in our code. We could define a macro that >>>> automatically retries upon SQLITE_BUSY. >>> >>> That would limit the issue to the first timeout span: for that short >>> time the scheduler would be blocked. I think a timeout of 0 would be >>> better. >> >> Yes, 0 is an acceptable =E2=80=9Csmall value.=E2=80=9D ;-) Perhaps 100= ms would be >> acceptable if the situation is rare enough, dunno. >> >>> Another solution would be to serialize all the database accesses as we >>> do already with the url handler, and stop using the SQLITE >>> multithreading features. It would probably make the code simpler >>> because we would use the same paradigm everywhere, and we would avoid >>> looping until SQLITE isn't busy at each request. >> >> In essence we=E2=80=99d introduce a =E2=80=9Cdatabase server=E2=80=9D ru= nning as a fiber, and >> everyone would talk to that server. > > Why not as a thread? There would be only one thread dedicated to SQLITE > (thus adding a constant overhead), and that would prevent the Scheduler > from being bloqued by long SQLite queries. Right, if we want to have a big timeout, it should be a thread, not a fiber. Sorry for the confusion. >> I considered doing that before but then though sqlite would probably be >> able to do better than this, but I don=E2=80=99t know. > > And it's hard to test... > >> What=E2=80=99s a bit annoying with switching to a database server model = is that >> we=E2=80=99d need to adapt every call site. >> >> Thoughts? > > I'm a tiny bit in favor of switching to a database server model because > it's more consistent overall (we already use it for the url handler) and > the scheduler wouldn't be bloqued by long SQLite queries. > > It's annoying to adapt every call site, but it would be done once and > for all. :-) Indeed. If you want to take that route, that=E2=80=99s fine with me! Thank you, Ludo=E2=80=99. From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: [PATCH 1/2] utils: Avoid deadlock when WITH-CRITICAL-SECTION calls are nested. References: <87k1ponc62.fsf@lassieur.org> In-Reply-To: <87k1ponc62.fsf@lassieur.org> Resent-From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 06 Aug 2018 19:29:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153358368723239 (code B ref 32234); Mon, 06 Aug 2018 19:29:02 +0000 Received: (at 32234) by debbugs.gnu.org; 6 Aug 2018 19:28:07 +0000 Received: from localhost ([127.0.0.1]:42899 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fmlAk-00062l-Qv for submit@debbugs.gnu.org; Mon, 06 Aug 2018 15:28:07 -0400 Received: from mail.lassieur.org ([83.152.10.219]:33008) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fmlAj-00062b-1q for 32234@debbugs.gnu.org; Mon, 06 Aug 2018 15:28:05 -0400 Received: from localhost.localdomain (88.191.118.83 [88.191.118.83]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id d69003a8 (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256:NO) for <32234@debbugs.gnu.org>; Mon, 6 Aug 2018 19:28:03 +0000 (UTC) From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Date: Mon, 6 Aug 2018 21:27:35 +0200 Message-Id: <20180806192736.1747-1-clement@lassieur.org> X-Mailer: git-send-email 2.18.0 X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) * src/cuirass/utils.scm (%critical-section-args): New parameter. (make-critical-section): Put ARGS into a parameter, so that CALL-WITH-CRITICAL-SECTION knows when it's called from the critical section. In that case it would just apply PROC to ARGS. (call-with-critical-section): If already in the critical section, apply PROC to %CRITICAL-SECTION-ARGS instead of sending the message through the critical section channel. --- src/cuirass/utils.scm | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/cuirass/utils.scm b/src/cuirass/utils.scm index 9e9ac36..6083890 100644 --- a/src/cuirass/utils.scm +++ b/src/cuirass/utils.scm @@ -94,6 +94,9 @@ delimited continuations and fibers." (conclusion) (apply throw args))))) +(define %critical-section-args + (make-parameter #f)) + (define (make-critical-section . args) "Return a channel used to implement a critical section. That channel can then be passed to 'join-critical-section', which will ensure sequential @@ -104,19 +107,23 @@ dedicated fiber." (let ((channel (make-channel))) (spawn-fiber (lambda () - (let loop () - (match (get-message channel) - (((? channel? reply) . (? procedure? proc)) - (put-message reply (apply proc args)))) - (loop)))) + (parameterize ((%critical-section-args args)) + (let loop () + (match (get-message channel) + (((? channel? reply) . (? procedure? proc)) + (put-message reply (apply proc args)))) + (loop))))) channel)) (define (call-with-critical-section channel proc) - "Call PROC in the critical section corresponding to CHANNEL. Return the -result of PROC." - (let ((reply (make-channel))) - (put-message channel (cons reply proc)) - (get-message reply))) + "Send PROC to the critical section through CHANNEL. Return the result of +PROC. If already in the critical section, call PROC immediately." + (let ((args (%critical-section-args))) + (if args + (apply proc args) + (let ((reply (make-channel))) + (put-message channel (cons reply proc)) + (get-message reply))))) (define-syntax-rule (with-critical-section channel (vars ...) exp ...) "Evaluate EXP... in the critical section corresponding to CHANNEL. -- 2.18.0 From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Resent-From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 06 Aug 2018 19:29:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153358369223256 (code B ref 32234); Mon, 06 Aug 2018 19:29:02 +0000 Received: (at 32234) by debbugs.gnu.org; 6 Aug 2018 19:28:12 +0000 Received: from localhost ([127.0.0.1]:42901 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fmlAo-000630-7N for submit@debbugs.gnu.org; Mon, 06 Aug 2018 15:28:12 -0400 Received: from mail.lassieur.org ([83.152.10.219]:33008) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fmlAj-00062b-QU for 32234@debbugs.gnu.org; Mon, 06 Aug 2018 15:28:08 -0400 Received: from localhost.localdomain (88.191.118.83 [88.191.118.83]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id 0fd4eac6 (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256:NO) for <32234@debbugs.gnu.org>; Mon, 6 Aug 2018 19:28:03 +0000 (UTC) From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Date: Mon, 6 Aug 2018 21:27:36 +0200 Message-Id: <20180806192736.1747-2-clement@lassieur.org> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20180806192736.1747-1-clement@lassieur.org> References: <20180806192736.1747-1-clement@lassieur.org> X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Fixes . * bin/cuirass.in (main): Keep only one WITH-DATABASE call around all fibers. Remove all DB arguments. * src/cuirass/base.scm (evaluate, update-build-statuses!, spawn-builds, handle-build-event, build-packages): Remove all DB arguments. (clear-build-queue, cancel-old-builds): Wrap in WITH-DB-CRITICAL-SECTION, remove all DB arguments. (restart-builds): Remove the NON-BLOCKING call, remove all DB arguments. (process-specs): Remove all DB arguments, remove the WITH-DATABASE call. * src/cuirass/database.scm (%db-channel): New parameter. (with-db-critical-section): New macro. (db-add-input, db-add-specification, db-get-inputs, db-get-specifications, db-add-evaluation, db-add-build, db-update-build-status!, db-get-outputs, db-get-builds, db-get-build, db-get-pending-derivations, db-get-stamp, db-add-stamp, db-get-evaluations, db-get-evaluations-build-summary, db-get-evaluations-id-min, db-get-evaluations-id-max, db-get-builds-min, db-get-builds-max): Wrap in WITH-DB-CRITICAL-SECTION, remove all DB arguments. (with-database): Wrap BODY in PARAMETERIZE form that sets %DB-CHANNEL to the channel returned by MAKE-CRITICAL-SECTION. * src/cuirass/http.scm (handle-build-request, handle-builds-request): Remove all DB arguments. (url-handler): Remove all DB arguments, remove the DB-CHANNEL state, remove the WITH-CRITICAL-SECTION calls. (run-cuirass-server): Remove the DB arguments, remove the MAKE-CRITICAL-SECTION call. * src/cuirass/utils.scm (make-critical-section): Replace SPAWN-FIBER with CALL-WITH-NEW-THREAD. Wrap body in PARAMETERIZE form that clears CURRENT-FIBER. * tests/database.scm (with-temporary-database, db-add-specification, db-add-build, db-update-build-status!, db-get-builds, db-get-pending-derivations): Remove the DB arguments. (db-init): Set the %DB-CHANNEL parameter to the channel returned by MAKE-CRITICAL-SECTION, and return #t. (database): Set %DB-CHANNEL to #f during cleanup. * tests/http.scm (db-init): Set the %DB-CHANNEL parameter to the channel returned by MAKE-CRITICAL-SECTION, and return #t. (cuirass-run, fill-db): Remove the DB arguments. (http): Set %DB-CHANNEL to #f during cleanup. --- bin/cuirass.in | 23 +- src/cuirass/base.scm | 94 +++---- src/cuirass/database.scm | 550 +++++++++++++++++++++------------------ src/cuirass/http.scm | 136 +++++----- src/cuirass/utils.scm | 23 +- tests/database.scm | 103 ++++---- tests/http.scm | 21 +- 7 files changed, 494 insertions(+), 456 deletions(-) diff --git a/bin/cuirass.in b/bin/cuirass.in index 11eb975..d30f788 100644 --- a/bin/cuirass.in +++ b/bin/cuirass.in @@ -115,19 +115,19 @@ exec ${GUILE:-@GUILE@} --no-auto-compile -e main -s "$0" "$@" (log-message "running Fibers on ~a kernel threads" threads) (run-fibers (lambda () - (with-database db + (with-database (and specfile (let ((new-specs (save-module-excursion (lambda () (set-current-module (make-user-module '())) (primitive-load specfile))))) - (for-each (lambda (spec) (db-add-specification db spec)) + (for-each (lambda (spec) (db-add-specification spec)) new-specs))) (if one-shot? - (process-specs db (db-get-specifications db)) + (process-specs (db-get-specifications)) (let ((exit-channel (make-channel))) - (clear-build-queue db) + (clear-build-queue) ;; First off, restart builds that had not completed or ;; were not even started on a previous run. @@ -135,25 +135,22 @@ exec ${GUILE:-@GUILE@} --no-auto-compile -e main -s "$0" "$@" (essential-task 'restart-builds exit-channel (lambda () - (with-database db - (restart-builds db))))) + (restart-builds)))) (spawn-fiber (essential-task 'build exit-channel (lambda () - (with-database db - (while #t - (process-specs db (db-get-specifications db)) - (log-message "next evaluation in ~a seconds" interval) - (sleep interval)))))) + (while #t + (process-specs (db-get-specifications)) + (log-message "next evaluation in ~a seconds" interval) + (sleep interval))))) (spawn-fiber (essential-task 'web-server exit-channel (lambda () - (with-database db - (run-cuirass-server db #:host host #:port port)))) + (run-cuirass-server #:host host #:port port))) #:parallel? #t) (spawn-fiber diff --git a/src/cuirass/base.scm b/src/cuirass/base.scm index 26a5996..abbdb7b 100644 --- a/src/cuirass/base.scm +++ b/src/cuirass/base.scm @@ -248,7 +248,7 @@ fibers." (logior (@ (fibers epoll) EPOLLERR) (@ (fibers epoll) EPOLLHUP))))) -(define (evaluate store db spec checkouts commits) +(define (evaluate store spec checkouts commits) "Evaluate and build package derivations defined in SPEC, using CHECKOUTS. Return a list of jobs." (define (augment-job job eval-id) @@ -277,8 +277,8 @@ Return a list of jobs." (('evaluation jobs) (let* ((spec-name (assq-ref spec #:name)) (eval-id (db-add-evaluation - db `((#:specification . ,spec-name) - (#:commits . ,commits))))) + `((#:specification . ,spec-name) + (#:commits . ,commits))))) (log-message "created evaluation ~a for '~a'" eval-id spec-name) (map (lambda (job) (augment-job job eval-id)) @@ -368,7 +368,7 @@ Essentially this procedure inverts the inversion-of-control that ;; Our shuffling algorithm is simple: we sort by .drv file name. :-) (sort drv stringoutput-paths drv) (((_ . outputs) ...) (if (any (cut valid-path? store <>) outputs) - (db-update-build-status! db drv (build-status succeeded)) - (db-update-build-status! db drv (build-status failed)))))) + (db-update-build-status! drv (build-status succeeded)) + (db-update-build-status! drv (build-status failed)))))) (for-each update! lst)) @@ -393,10 +393,11 @@ and returns the values RESULTS." (print-exception (current-error-port) frame key args) (apply values results))))) -(define* (spawn-builds store db drv +(define* (spawn-builds store drv #:key (max-batch-size 200)) - "Build the derivations listed in DRV, updating DB as builds complete. -Derivations are submitted in batches of at most MAX-BATCH-SIZE items." + "Build the derivations listed in DRV, updating the database as builds +complete. Derivations are submitted in batches of at most MAX-BATCH-SIZE +items." ;; XXX: We want to pass 'build-derivations' as many derivations at once so ;; we benefit from as much parallelism as possible (we must be using ;; #:keep-going? #t). @@ -444,7 +445,7 @@ Derivations are submitted in batches of at most MAX-BATCH-SIZE items." ;; from PORT and eventually close it. (catch #t (lambda () - (handle-build-event db event)) + (handle-build-event event)) (exception-reporter state))) #t) (close-port port) @@ -455,14 +456,14 @@ Derivations are submitted in batches of at most MAX-BATCH-SIZE items." ;; derivations were built "behind our back", in which case ;; 'build-derivations' doesn't actually do anything and ;; 'handle-build-event' doesn't see any event. Because of that, - ;; adjust DB here. - (update-build-statuses! store db batch) + ;; adjust the database here. + (update-build-statuses! store batch) (loop rest (max (- count max-batch-size) 0)))))) -(define* (handle-build-event db event) +(define* (handle-build-event event) "Handle EVENT, a build event sexp as produced by 'build-event-output-port', -updating DB accordingly." +updating the database accordingly." (define (valid? file) ;; FIXME: Sometimes we might get bogus events due to the interleaving of ;; build messages. This procedure prevents us from propagating the bogus @@ -475,7 +476,7 @@ updating DB accordingly." (if (valid? drv) (begin (log-message "build started: '~a'" drv) - (db-update-build-status! db drv (build-status started))) + (db-update-build-status! drv (build-status started))) (log-message "bogus build-started event for '~a'" drv))) (('build-remote drv host _ ...) (log-message "'~a' offloaded to '~a'" drv host)) @@ -483,13 +484,13 @@ updating DB accordingly." (if (valid? drv) (begin (log-message "build succeeded: '~a'" drv) - (db-update-build-status! db drv (build-status succeeded))) + (db-update-build-status! drv (build-status succeeded))) (log-message "bogus build-succeeded event for '~a'" drv))) (('build-failed drv _ ...) (if (valid? drv) (begin (log-message "build failed: '~a'" drv) - (db-update-build-status! db drv (build-status failed))) + (db-update-build-status! drv (build-status failed))) (log-message "bogus build-failed event for '~a'" drv))) (('substituter-started item _ ...) (log-message "substituter started: '~a'" item)) @@ -503,42 +504,42 @@ updating DB accordingly." (string=? (assq-ref build1 #:derivation) (assq-ref build2 #:derivation))) -(define (clear-build-queue db) - "Reset the status of builds in DB that are marked as \"started\". This -procedure is meant to be called at startup." +(define (clear-build-queue) + "Reset the status of builds in the database that are marked as \"started\". +This procedure is meant to be called at startup." (log-message "marking stale builds as \"scheduled\"...") - (sqlite-exec db "UPDATE Builds SET status = -2 WHERE status = -1;")) + (with-db-critical-section db + (sqlite-exec db "UPDATE Builds SET status = -2 WHERE status = -1;"))) -(define (cancel-old-builds db age) +(define (cancel-old-builds age) "Cancel builds older than AGE seconds." (log-message "canceling builds older than ~a seconds..." age) - (sqlite-exec db - "UPDATE Builds SET status = 4 WHERE status = -2 AND timestamp < " - (- (time-second (current-time time-utc)) age) - ";")) - -(define (restart-builds db) - "Restart builds whose status in DB is \"pending\" (scheduled or started)." + (with-db-critical-section db + (sqlite-exec + db "UPDATE Builds SET status = 4 WHERE status = -2 AND timestamp < " + (- (time-second (current-time time-utc)) age) ";"))) + +(define (restart-builds) + "Restart builds whose status in the database is \"pending\" (scheduled or +started)." (with-store store - ;; Note: On a big database, 'db-get-pending-derivations' can take a couple - ;; of minutes, hence 'non-blocking'. (log-message "retrieving list of pending builds...") (let*-values (((valid stale) (partition (cut valid-path? store <>) - (non-blocking (db-get-pending-derivations db))))) + (db-get-pending-derivations)))) ;; We cannot restart builds listed in STALE, so mark them as canceled. (log-message "canceling ~a stale builds" (length stale)) (for-each (lambda (drv) - (db-update-build-status! db drv (build-status canceled))) + (db-update-build-status! drv (build-status canceled))) stale) ;; Those in VALID can be restarted. If some of them were built in the ;; meantime behind our back, that's fine: 'spawn-builds' will DTRT. (log-message "restarting ~a pending builds" (length valid)) - (spawn-builds store db valid) + (spawn-builds store valid) (log-message "done with restarted builds")))) -(define (build-packages store db jobs) +(define (build-packages store jobs) "Build JOBS and return a list of Build results." (define (register job) (let* ((name (assq-ref job #:job-name)) @@ -570,14 +571,14 @@ procedure is meant to be called at startup." (#:timestamp . ,cur-time) (#:starttime . 0) (#:stoptime . 0)))) - (db-add-build db build)))) + (db-add-build build)))) (define derivations (filter-map register jobs)) - (spawn-builds store db derivations) + (spawn-builds store derivations) - (let* ((results (filter-map (cut db-get-build db <>) derivations)) + (let* ((results (filter-map (cut db-get-build <>) derivations)) (status (map (cut assq-ref <> #:status) results)) (success (count (lambda (status) (= status (build-status succeeded))) @@ -651,11 +652,11 @@ procedure is meant to be called at startup." checkout) results))) -(define (process-specs db jobspecs) - "Evaluate and build JOBSPECS and store results in DB." +(define (process-specs jobspecs) + "Evaluate and build JOBSPECS and store results in the database." (define (process spec) (with-store store - (let* ((stamp (db-get-stamp db spec)) + (let* ((stamp (db-get-stamp spec)) (name (assoc-ref spec #:name)) (checkouts (fetch-inputs spec)) (commits (map (cut assq-ref <> #:commit) checkouts)) @@ -663,7 +664,7 @@ procedure is meant to be called at startup." (unless (equal? commits-str stamp) ;; Immediately mark SPEC's INPUTS as being processed so we don't ;; spawn a concurrent evaluation of that same commit. - (db-add-stamp db spec commits-str) + (db-add-stamp spec commits-str) (compile-checkouts spec (filter compile? checkouts)) (spawn-fiber (lambda () @@ -674,11 +675,10 @@ procedure is meant to be called at startup." (log-message "evaluating spec '~a': stamp ~s different from ~s" name commits-str stamp) (with-store store - (with-database db - (let ((jobs (evaluate store db spec checkouts commits))) - (log-message "building ~a jobs for '~a'" - (length jobs) name) - (build-packages store db jobs))))))) + (let ((jobs (evaluate store spec checkouts commits))) + (log-message "building ~a jobs for '~a'" + (length jobs) name) + (build-packages store jobs)))))) ;; 'spawn-fiber' returns zero values but we need one. *unspecified*)))) diff --git a/src/cuirass/database.scm b/src/cuirass/database.scm index 7788ac9..5cf84aa 100644 --- a/src/cuirass/database.scm +++ b/src/cuirass/database.scm @@ -59,7 +59,9 @@ ;; Parameters. %package-database %package-schema-file + %db-channel ;; Macros. + with-db-critical-section with-database)) (define (%sqlite-exec db sql . args) @@ -139,6 +141,16 @@ question marks matches the number of arguments to bind." (string-append %datadir "/" %package)) "/sql"))) +(define %db-channel + (make-parameter #f)) + +(define-syntax-rule (with-db-critical-section db exp ...) + "Evaluate EXP... in the critical section corresponding to %DB-CHANNEL. +DB is bound to the argument of that critical section: the database +connection." + (call-with-critical-section (%db-channel) + (lambda (db) exp ...))) + (define (read-sql-file file-name) "Return a list of string containing SQL instructions from FILE-NAME." (call-with-input-file file-name @@ -238,92 +250,111 @@ database object." (vector-ref (car (sqlite-exec db "SELECT last_insert_rowid();")) 0)) -(define (db-add-input db spec-name input) - (sqlite-exec db "\ +(define (db-add-input spec-name input) + (with-db-critical-section db + (sqlite-exec db "\ INSERT OR IGNORE INTO Inputs (specification, name, url, load_path, branch, \ tag, revision, no_compile_p) VALUES (" - spec-name ", " - (assq-ref input #:name) ", " - (assq-ref input #:url) ", " - (assq-ref input #:load-path) ", " - (assq-ref input #:branch) ", " - (assq-ref input #:tag) ", " - (assq-ref input #:commit) ", " - (if (assq-ref input #:no-compile?) 1 0) ");") - (last-insert-rowid db)) - -(define (db-add-specification db spec) - "Store SPEC in database DB. SPEC inputs are stored in the INPUTS table." - (sqlite-exec db "\ + spec-name ", " + (assq-ref input #:name) ", " + (assq-ref input #:url) ", " + (assq-ref input #:load-path) ", " + (assq-ref input #:branch) ", " + (assq-ref input #:tag) ", " + (assq-ref input #:commit) ", " + (if (assq-ref input #:no-compile?) 1 0) ");") + (last-insert-rowid db))) + +(define (db-add-specification spec) + "Store SPEC in database the database. SPEC inputs are stored in the INPUTS +table." + (with-db-critical-section db + (sqlite-exec db "\ INSERT OR IGNORE INTO Specifications (name, load_path_inputs, \ package_path_inputs, proc_input, proc_file, proc, proc_args) \ VALUES (" - (assq-ref spec #:name) ", " - (assq-ref spec #:load-path-inputs) ", " - (assq-ref spec #:package-path-inputs)", " - (assq-ref spec #:proc-input) ", " - (assq-ref spec #:proc-file) ", " - (symbol->string (assq-ref spec #:proc)) ", " - (assq-ref spec #:proc-args) ");") - (let ((spec-id (last-insert-rowid db))) - (for-each (lambda (input) - (db-add-input db (assq-ref spec #:name) input)) - (assq-ref spec #:inputs)) - spec-id)) - -(define (db-get-inputs db spec-name) - (let loop ((rows (sqlite-exec db "SELECT * FROM Inputs WHERE specification=" - spec-name ";")) - (inputs '())) - (match rows - (() inputs) - ((#(specification name url load-path branch tag revision no-compile-p) - . rest) - (loop rest - (cons `((#:name . ,name) - (#:url . ,url) - (#:load-path . ,load-path) - (#:branch . ,branch) - (#:tag . ,tag) - (#:commit . ,revision) - (#:no-compile? . ,(positive? no-compile-p))) - inputs)))))) - -(define (db-get-specifications db) - (let loop ((rows (sqlite-exec db "SELECT * FROM Specifications;")) - (specs '())) - (match rows - (() specs) - ((#(name load-path-inputs package-path-inputs proc-input proc-file proc - proc-args) - . rest) - (loop rest - (cons `((#:name . ,name) - (#:load-path-inputs . - ,(with-input-from-string load-path-inputs read)) - (#:package-path-inputs . - ,(with-input-from-string package-path-inputs read)) - (#:proc-input . ,proc-input) - (#:proc-file . ,proc-file) - (#:proc . ,(with-input-from-string proc read)) - (#:proc-args . ,(with-input-from-string proc-args read)) - (#:inputs . ,(db-get-inputs db name))) - specs)))))) - -(define (db-add-evaluation db eval) - (sqlite-exec db "\ + (assq-ref spec #:name) ", " + (assq-ref spec #:load-path-inputs) ", " + (assq-ref spec #:package-path-inputs) ", " + (assq-ref spec #:proc-input) ", " + (assq-ref spec #:proc-file) ", " + (symbol->string (assq-ref spec #:proc)) ", " + (assq-ref spec #:proc-args) ");") + (let ((spec-id (last-insert-rowid db))) + (for-each (lambda (input) + (db-add-input (assq-ref spec #:name) input)) + (assq-ref spec #:inputs)) + spec-id))) + +(define (db-get-inputs spec-name) + (with-db-critical-section db + (let loop ((rows (sqlite-exec + db "SELECT * FROM Inputs WHERE specification=" + spec-name ";")) + (inputs '())) + (match rows + (() inputs) + ((#(specification name url load-path branch tag revision no-compile-p) + . rest) + (loop rest + (cons `((#:name . ,name) + (#:url . ,url) + (#:load-path . ,load-path) + (#:branch . ,branch) + (#:tag . ,tag) + (#:commit . ,revision) + (#:no-compile? . ,(positive? no-compile-p))) + inputs))))))) + +(define (db-get-specifications) + (with-db-critical-section db + (let loop ((rows (sqlite-exec db "SELECT * FROM Specifications;")) + (specs '())) + (match rows + (() specs) + ((#(name load-path-inputs package-path-inputs proc-input proc-file proc + proc-args) + . rest) + (loop rest + (cons `((#:name . ,name) + (#:load-path-inputs . + ,(with-input-from-string load-path-inputs read)) + (#:package-path-inputs . + ,(with-input-from-string package-path-inputs read)) + (#:proc-input . ,proc-input) + (#:proc-file . ,proc-file) + (#:proc . ,(with-input-from-string proc read)) + (#:proc-args . ,(with-input-from-string proc-args read)) + (#:inputs . ,(db-get-inputs name))) + specs))))))) + +(define (db-add-evaluation eval) + (with-db-critical-section db + (sqlite-exec db "\ INSERT INTO Evaluations (specification, commits) VALUES (" - (assq-ref eval #:specification) ", " - (string-join (assq-ref eval #:commits)) ");") - (last-insert-rowid db)) + (assq-ref eval #:specification) ", " + (string-join (assq-ref eval #:commits)) ");") + (last-insert-rowid db))) -(define-syntax-rule (with-database db body ...) - "Run BODY with a connection to the database which is bound to DB in BODY." +(define-syntax-rule (with-database body ...) + "Run BODY with %DB-CHANNEL being dynamically bound to a channel implementing +a critical section that allows database operations to be serialized." ;; XXX: We don't install an unwind handler to play well with delimited ;; continuations and fibers. But as a consequence, we leak DB when BODY ;; raises an exception. (let ((db (db-open))) - (unwind-protect body ... (db-close db)))) + (unwind-protect + ;; Process database queries sequentially in a thread. We need this + ;; because otherwise we would need to use the SQLite multithreading + ;; feature for which it is required to wait until the database is + ;; available, and the waiting would happen in non-cooperative and + ;; non-resumable code that blocks the fibers scheduler. Now the database + ;; access blocks on PUT-MESSAGE, which allows the scheduler to schedule + ;; another fiber. Also, creating one new handle for each request would + ;; be costly and may defeat statement caching. + (parameterize ((%db-channel (make-critical-section db))) + body ...) + (db-close db)))) (define* (read-quoted-string #:optional (port (current-input-port))) "Read all of the characters out of PORT and return them as a SQL quoted @@ -353,79 +384,84 @@ string." (failed-other 3) (canceled 4)) -(define (db-add-build db build) - "Store BUILD in database DB. BUILD eventual outputs are stored -in the OUTPUTS table." - (catch 'sqlite-error - (lambda () - (sqlite-exec db " +(define (db-add-build build) + "Store BUILD in database the database. BUILD eventual outputs are stored in +the OUTPUTS table." + (with-db-critical-section db + (catch 'sqlite-error + (lambda () + (sqlite-exec db " INSERT INTO Builds (derivation, evaluation, job_name, system, nix_name, log, status, timestamp, starttime, stoptime) VALUES (" - (assq-ref build #:derivation) ", " - (assq-ref build #:eval-id) ", " - (assq-ref build #:job-name) ", " - (assq-ref build #:system) ", " - (assq-ref build #:nix-name) ", " - (assq-ref build #:log) ", " - (or (assq-ref build #:status) - (build-status scheduled)) ", " - (or (assq-ref build #:timestamp) 0) ", " - (or (assq-ref build #:starttime) 0) ", " - (or (assq-ref build #:stoptime) 0) ");") - (let ((derivation (assq-ref build #:derivation))) - (for-each (lambda (output) - (match output - ((name . path) - (sqlite-exec db "\ + (assq-ref build #:derivation) ", " + (assq-ref build #:eval-id) ", " + (assq-ref build #:job-name) ", " + (assq-ref build #:system) ", " + (assq-ref build #:nix-name) ", " + (assq-ref build #:log) ", " + (or (assq-ref build #:status) + (build-status scheduled)) ", " + (or (assq-ref build #:timestamp) 0) ", " + (or (assq-ref build #:starttime) 0) ", " + (or (assq-ref build #:stoptime) 0) ");") + (let ((derivation (assq-ref build #:derivation))) + (for-each (lambda (output) + (match output + ((name . path) + (sqlite-exec db "\ INSERT INTO Outputs (derivation, name, path) VALUES (" - derivation ", " name ", " path ");")))) - (assq-ref build #:outputs)) - derivation)) - (lambda (key who code message . rest) - ;; If we get a unique-constraint-failed error, that means we have - ;; already inserted the same build. That happens when several jobs - ;; produce the same derivation, and we can ignore it. - (if (= code SQLITE_CONSTRAINT_PRIMARYKEY) - #f - (apply throw key who code rest))))) - -(define* (db-update-build-status! db drv status #:key log-file) - "Update DB so that DRV's status is STATUS. This also updates the + derivation ", " name ", " path ");")))) + (assq-ref build #:outputs)) + derivation)) + (lambda (key who code message . rest) + ;; If we get a unique-constraint-failed error, that means we have + ;; already inserted the same build. That happens when several jobs + ;; produce the same derivation, and we can ignore it. + (if (= code SQLITE_CONSTRAINT_PRIMARYKEY) + #f + (apply throw key who code rest)))))) + +(define* (db-update-build-status! drv status #:key log-file) + "Update the database so that DRV's status is STATUS. This also updates the 'starttime' or 'stoptime' fields. If LOG-FILE is true, record it as the build log file for DRV." (define now (time-second (current-time time-utc))) - (if (= status (build-status started)) - (sqlite-exec db "UPDATE Builds SET starttime=" now ", status=" - status "WHERE derivation=" drv ";") - - ;; Update only if we're switching to a different status; otherwise leave - ;; things unchanged. This ensures that 'stoptime' remains valid and - ;; doesn't change every time we mark DRV as 'succeeded' several times in - ;; a row, for instance. - (if log-file - (sqlite-exec db "UPDATE Builds SET stoptime=" now - ", status=" status ", log=" log-file - "WHERE derivation=" drv "AND status != " status ";") - (sqlite-exec db "UPDATE Builds SET stoptime=" now - ", status=" status - "WHERE derivation=" drv " AND status != " status ";")))) - -(define (db-get-outputs db derivation) - "Retrieve the OUTPUTS of the build identified by DERIVATION in DB database." - (let loop ((rows - (sqlite-exec db "SELECT name, path FROM Outputs + (with-db-critical-section db + (if (= status (build-status started)) + (sqlite-exec db "UPDATE Builds SET starttime=" now ", status=" + status "WHERE derivation=" drv ";") + + ;; Update only if we're switching to a different status; otherwise + ;; leave things unchanged. This ensures that 'stoptime' remains valid + ;; and doesn't change every time we mark DRV as 'succeeded' several + ;; times in a row, for instance. + (if log-file + (sqlite-exec db "UPDATE Builds SET stoptime=" now + ", status=" status ", log=" log-file + "WHERE derivation=" drv "AND status != " status ";") + (sqlite-exec db "UPDATE Builds SET stoptime=" now + ", status=" status + "WHERE derivation=" drv " AND status != " status + ";"))))) + +(define (db-get-outputs derivation) + "Retrieve the OUTPUTS of the build identified by DERIVATION in the +database." + (with-db-critical-section db + (let loop ((rows + (sqlite-exec db "SELECT name, path FROM Outputs WHERE derivation =" derivation ";")) - (outputs '())) - (match rows - (() outputs) - ((#(name path) - . rest) - (loop rest - (cons `(,name . ((#:path . ,path))) - outputs)))))) + (outputs '())) + (match rows + (() outputs) + ((#(name path) + . rest) + (loop rest + (cons `(,name . ((#:path . ,path))) + outputs))))))) (define (filters->order filters) (match (assq 'order filters) @@ -440,12 +476,13 @@ WHERE derivation =" derivation ";")) (('order . 'status+submission-time) "status DESC, timestamp DESC") (_ "rowid DESC"))) -(define (db-get-builds db filters) - "Retrieve all builds in database DB which are matched by given FILTERS. +(define (db-get-builds filters) + "Retrieve all builds in the database which are matched by given FILTERS. FILTERS is an assoc list whose possible keys are 'derivation | 'id | 'jobset | 'job | 'system | 'nr | 'order | 'status | 'evaluation." - (let* ((order (filters->order filters)) - (stmt-text (format #f "SELECT * FROM ( + (with-db-critical-section db + (let* ((order (filters->order filters)) + (stmt-text (format #f "SELECT * FROM ( SELECT Builds.derivation, Builds.rowid, Builds.timestamp, Builds.starttime, Builds.stoptime, Builds.log, Builds.status, Builds.job_name, Builds.system, Builds.nix_name, Specifications.name @@ -475,93 +512,99 @@ CASE WHEN :borderlowtime IS NULL END DESC LIMIT :nr) ORDER BY ~a, rowid ASC;" order)) - (stmt (sqlite-prepare db stmt-text #:cache? #t))) - (sqlite-bind-arguments - stmt - #:derivation (assq-ref filters 'derivation) - #:id (assq-ref filters 'id) - #:jobset (assq-ref filters 'jobset) - #:job (assq-ref filters 'job) - #:evaluation (assq-ref filters 'evaluation) - #:system (assq-ref filters 'system) - #:status (and=> (assq-ref filters 'status) object->string) - #:borderlowid (assq-ref filters 'border-low-id) - #:borderhighid (assq-ref filters 'border-high-id) - #:borderlowtime (assq-ref filters 'border-low-time) - #:borderhightime (assq-ref filters 'border-high-time) - #:nr (match (assq-ref filters 'nr) - (#f -1) - (x x))) - (sqlite-reset stmt) - (let loop ((rows (sqlite-fold-right cons '() stmt)) - (builds '())) - (match rows - (() (reverse builds)) - ((#(derivation id timestamp starttime stoptime log status job-name - system nix-name specification) . rest) - (loop rest - (cons `((#:derivation . ,derivation) - (#:id . ,id) - (#:timestamp . ,timestamp) - (#:starttime . ,starttime) - (#:stoptime . ,stoptime) - (#:log . ,log) - (#:status . ,status) - (#:job-name . ,job-name) - (#:system . ,system) - (#:nix-name . ,nix-name) - (#:specification . ,specification) - (#:outputs . ,(db-get-outputs db derivation))) - builds))))))) - -(define (db-get-build db derivation-or-id) - "Retrieve a build in database DB which corresponds to DERIVATION-OR-ID." - (let ((key (if (number? derivation-or-id) 'id 'derivation))) - (match (db-get-builds db `((,key . ,derivation-or-id))) - ((build) - build) - (() #f)))) - -(define (db-get-pending-derivations db) + (stmt (sqlite-prepare db stmt-text #:cache? #t))) + (sqlite-bind-arguments + stmt + #:derivation (assq-ref filters 'derivation) + #:id (assq-ref filters 'id) + #:jobset (assq-ref filters 'jobset) + #:job (assq-ref filters 'job) + #:evaluation (assq-ref filters 'evaluation) + #:system (assq-ref filters 'system) + #:status (and=> (assq-ref filters 'status) object->string) + #:borderlowid (assq-ref filters 'border-low-id) + #:borderhighid (assq-ref filters 'border-high-id) + #:borderlowtime (assq-ref filters 'border-low-time) + #:borderhightime (assq-ref filters 'border-high-time) + #:nr (match (assq-ref filters 'nr) + (#f -1) + (x x))) + (sqlite-reset stmt) + (let loop ((rows (sqlite-fold-right cons '() stmt)) + (builds '())) + (match rows + (() (reverse builds)) + ((#(derivation id timestamp starttime stoptime log status job-name + system nix-name specification) . rest) + (loop rest + (cons `((#:derivation . ,derivation) + (#:id . ,id) + (#:timestamp . ,timestamp) + (#:starttime . ,starttime) + (#:stoptime . ,stoptime) + (#:log . ,log) + (#:status . ,status) + (#:job-name . ,job-name) + (#:system . ,system) + (#:nix-name . ,nix-name) + (#:specification . ,specification) + (#:outputs . ,(db-get-outputs derivation))) + builds)))))))) + +(define (db-get-build derivation-or-id) + "Retrieve a build in the database which corresponds to DERIVATION-OR-ID." + (with-db-critical-section db + (let ((key (if (number? derivation-or-id) 'id 'derivation))) + (match (db-get-builds `((,key . ,derivation-or-id))) + ((build) + build) + (() #f))))) + +(define (db-get-pending-derivations) "Return the list of derivation file names corresponding to pending builds in -DB. The returned list is guaranteed to not have any duplicates." - (map (match-lambda (#(drv) drv)) - (sqlite-exec db " -SELECT derivation FROM Builds WHERE Builds.status < 0;"))) - -(define (db-get-stamp db spec) - "Return a stamp corresponding to specification SPEC in database DB." - (let ((res (sqlite-exec db "SELECT * FROM Stamps WHERE specification=" - (assq-ref spec #:name) ";"))) - (match res - (() #f) - ((#(spec stamp)) stamp)))) - -(define (db-add-stamp db spec stamp) - "Associate STAMP to specification SPEC in database DB." - (if (db-get-stamp db spec) - (sqlite-exec db "UPDATE Stamps SET stamp=" stamp - "WHERE specification=" (assq-ref spec #:name) ";") - (sqlite-exec db "\ +the database. The returned list is guaranteed to not have any duplicates." + (with-db-critical-section db + (map (match-lambda (#(drv) drv)) + (sqlite-exec db " +SELECT derivation FROM Builds WHERE Builds.status < 0;")))) + +(define (db-get-stamp spec) + "Return a stamp corresponding to specification SPEC in the database." + (with-db-critical-section db + (let ((res (sqlite-exec db "SELECT * FROM Stamps WHERE specification=" + (assq-ref spec #:name) ";"))) + (match res + (() #f) + ((#(spec stamp)) stamp))))) + +(define (db-add-stamp spec stamp) + "Associate STAMP to specification SPEC in the database." + (with-db-critical-section db + (if (db-get-stamp spec) + (sqlite-exec db "UPDATE Stamps SET stamp=" stamp + "WHERE specification=" (assq-ref spec #:name) ";") + (sqlite-exec db "\ INSERT INTO Stamps (specification, stamp) VALUES (" - (assq-ref spec #:name) ", " stamp ");"))) + (assq-ref spec #:name) ", " stamp ");")))) -(define (db-get-evaluations db limit) - (let loop ((rows (sqlite-exec db "SELECT id, specification, commits +(define (db-get-evaluations limit) + (with-db-critical-section db + (let loop ((rows (sqlite-exec db "SELECT id, specification, commits FROM Evaluations ORDER BY id DESC LIMIT " limit ";")) - (evaluations '())) - (match rows - (() (reverse evaluations)) - ((#(id specification commits) - . rest) - (loop rest - (cons `((#:id . ,id) - (#:specification . ,specification) - (#:commits . ,(string-tokenize commits))) - evaluations)))))) - -(define (db-get-evaluations-build-summary db spec limit border-low border-high) - (let loop ((rows (sqlite-exec db " + (evaluations '())) + (match rows + (() (reverse evaluations)) + ((#(id specification commits) + . rest) + (loop rest + (cons `((#:id . ,id) + (#:specification . ,specification) + (#:commits . ,(string-tokenize commits))) + evaluations))))))) + +(define (db-get-evaluations-build-summary spec limit border-low border-high) + (with-db-critical-section db + (let loop ((rows (sqlite-exec db " SELECT E.id, E.commits, B.succeeded, B.failed, B.scheduled FROM (SELECT id, commits @@ -578,50 +621,59 @@ FROM Builds GROUP BY evaluation) B ON B.evaluation=E.id ORDER BY E.id ASC;")) - (evaluations '())) - (match rows - (() evaluations) - ((#(id commits succeeded failed scheduled) . rest) - (loop rest - (cons `((#:id . ,id) - (#:commits . ,commits) - (#:succeeded . ,(or succeeded 0)) - (#:failed . ,(or failed 0)) - (#:scheduled . ,(or scheduled 0))) - evaluations)))))) - -(define (db-get-evaluations-id-min db spec) + (evaluations '())) + (match rows + (() evaluations) + ((#(id commits succeeded failed scheduled) . rest) + (loop rest + (cons `((#:id . ,id) + (#:commits . ,commits) + (#:succeeded . ,(or succeeded 0)) + (#:failed . ,(or failed 0)) + (#:scheduled . ,(or scheduled 0))) + evaluations))))))) + +(define (db-get-evaluations-id-min spec) "Return the min id of evaluations for the given specification SPEC." - (let ((rows (sqlite-exec db " + (with-db-critical-section db + (let ((rows (sqlite-exec db " SELECT MIN(id) FROM Evaluations WHERE specification=" spec))) - (vector-ref (car rows) 0))) + (vector-ref (car rows) 0)))) -(define (db-get-evaluations-id-max db spec) +(define (db-get-evaluations-id-max spec) "Return the max id of evaluations for the given specification SPEC." - (let ((rows (sqlite-exec db " + (with-db-critical-section db + (let ((rows (sqlite-exec db " SELECT MAX(id) FROM Evaluations WHERE specification=" spec))) - (vector-ref (car rows) 0))) + (vector-ref (car rows) 0)))) -(define (db-get-builds-min db eval) +(define (db-get-builds-min eval) "Return the min build (stoptime, id) pair for the given evaluation EVAL." - (let ((rows (sqlite-exec db " + (with-db-critical-section db + (let ((rows (sqlite-exec db " SELECT stoptime, MIN(rowid) FROM (SELECT rowid, stoptime FROM Builds WHERE evaluation=" eval " AND stoptime = (SELECT MIN(stoptime) FROM Builds WHERE evaluation=" eval "))"))) - (vector->list (car rows)))) + (vector->list (car rows))))) -(define (db-get-builds-max db eval) +(define (db-get-builds-max eval) "Return the max build (stoptime, id) pair for the given evaluation EVAL." - (let ((rows (sqlite-exec db " + (with-db-critical-section db + (let ((rows (sqlite-exec db " SELECT stoptime, MAX(rowid) FROM (SELECT rowid, stoptime FROM Builds WHERE evaluation=" eval " AND stoptime = (SELECT MAX(stoptime) FROM Builds WHERE evaluation=" eval "))"))) - (vector->list (car rows)))) + (vector->list (car rows))))) + +;;; Local Variables: +;;; eval: (put 'with-db-critical-section 'scheme-indent-function 1) +;;; eval: (put 'with-database 'scheme-indent-function 0) +;;; End: diff --git a/src/cuirass/http.scm b/src/cuirass/http.scm index 16bbda0..d70517b 100644 --- a/src/cuirass/http.scm +++ b/src/cuirass/http.scm @@ -103,17 +103,17 @@ (#:releasename . #nil) (#:buildinputs_builds . #nil))) -(define (handle-build-request db build-id) - "Retrieve build identified by BUILD-ID over DB and convert it - to hydra format. Return #f is not build was found." - (let ((build (db-get-build db build-id))) +(define (handle-build-request build-id) + "Retrieve build identified by BUILD-ID over the database and convert it to +hydra format. Return #f is not build was found." + (let ((build (db-get-build build-id))) (and=> build build->hydra-build))) -(define (handle-builds-request db filters) - "Retrieve all builds matched by FILTERS in DB and convert them - to Hydra format." +(define (handle-builds-request filters) + "Retrieve all builds matched by FILTERS in the database and convert them to +Hydra format." (let ((builds (with-time-logging "builds request" - (db-get-builds db filters)))) + (db-get-builds filters)))) (map build->hydra-build builds))) (define (request-parameters request) @@ -146,10 +146,10 @@ (define (request-path-components request) (split-and-decode-uri-path (uri-path (request-uri request)))) -(define (url-handler request body db-channel) +(define (url-handler request body) - (define* (respond response #:key body (db-channel db-channel)) - (values response body db-channel)) + (define* (respond response #:key body) + (values response body #f)) (define-syntax-rule (respond-json body ...) (respond '((content-type . (application/json))) @@ -213,19 +213,14 @@ (request-path-components request) 'method-not-allowed) (((or "jobsets" "specifications") . rest) - (respond-json (object->json-string - (with-critical-section db-channel (db) - (db-get-specifications db))))) + (respond-json (object->json-string (db-get-specifications)))) (("build" build-id) - (let ((hydra-build - (with-critical-section db-channel (db) - (handle-build-request db (string->number build-id))))) + (let ((hydra-build (handle-build-request (string->number build-id)))) (if hydra-build (respond-json (object->json-string hydra-build)) (respond-build-not-found build-id)))) (("build" build-id "log" "raw") - (let ((build (with-critical-section db-channel (db) - (db-get-build db (string->number build-id))))) + (let ((build (db-get-build (string->number build-id)))) (if build (match (assq-ref build #:outputs) (((_ (#:path . (? string? output))) _ ...) @@ -250,9 +245,7 @@ ;; 'nr parameter is mandatory to limit query size. (nr (assq-ref params 'nr))) (if nr - (respond-json (object->json-string - (with-critical-section db-channel (db) - (db-get-evaluations db nr)))) + (respond-json (object->json-string (db-get-evaluations nr))) (respond-json-with-error 500 "Parameter not defined!")))) (("api" "latestbuilds") (let* ((params (request-parameters request)) @@ -262,10 +255,9 @@ ;; Limit results to builds that are "done". (respond-json (object->json-string - (with-critical-section db-channel (db) - (handle-builds-request db `((status . done) - ,@params - (order . finish-time)))))) + (handle-builds-request `((status . done) + ,@params + (order . finish-time))))) (respond-json-with-error 500 "Parameter not defined!")))) (("api" "queue") (let* ((params (request-parameters request)) @@ -276,77 +268,65 @@ (object->json-string ;; Use the 'status+submission-time' order so that builds in ;; 'running' state appear before builds in 'scheduled' state. - (with-critical-section db-channel (db) - (handle-builds-request db `((status . pending) - ,@params - (order . status+submission-time)))))) + (handle-builds-request `((status . pending) + ,@params + (order . status+submission-time))))) (respond-json-with-error 500 "Parameter not defined!")))) ('() (respond-html (html-page "Cuirass" - (specifications-table - (with-critical-section db-channel (db) - (db-get-specifications db)))))) + (specifications-table (db-get-specifications))))) (("jobset" name) (respond-html - (with-critical-section db-channel (db) - (let* ((evaluation-id-max (db-get-evaluations-id-max db name)) - (evaluation-id-min (db-get-evaluations-id-min db name)) - (params (request-parameters request)) - (border-high (assq-ref params 'border-high)) - (border-low (assq-ref params 'border-low)) - (evaluations (db-get-evaluations-build-summary db - name - %page-size - border-low - border-high))) - (html-page name (evaluation-info-table name - evaluations - evaluation-id-min - evaluation-id-max)))))) + (let* ((evaluation-id-max (db-get-evaluations-id-max name)) + (evaluation-id-min (db-get-evaluations-id-min name)) + (params (request-parameters request)) + (border-high (assq-ref params 'border-high)) + (border-low (assq-ref params 'border-low)) + (evaluations (db-get-evaluations-build-summary name + %page-size + border-low + border-high))) + (html-page name (evaluation-info-table name + evaluations + evaluation-id-min + evaluation-id-max))))) (("eval" id) (respond-html - (with-critical-section db-channel (db) - (let* ((builds-id-max (db-get-builds-max db id)) - (builds-id-min (db-get-builds-min db id)) - (params (request-parameters request)) - (border-high-time (assq-ref params 'border-high-time)) - (border-low-time (assq-ref params 'border-low-time)) - (border-high-id (assq-ref params 'border-high-id)) - (border-low-id (assq-ref params 'border-low-id))) - (html-page - "Evaluation" - (build-eval-table - (handle-builds-request db `((evaluation . ,id) - (nr . ,%page-size) - (order . finish-time+build-id) - (border-high-time . ,border-high-time) - (border-low-time . ,border-low-time) - (border-high-id . ,border-high-id) - (border-low-id . ,border-low-id))) - builds-id-min - builds-id-max)))))) + (let* ((builds-id-max (db-get-builds-max id)) + (builds-id-min (db-get-builds-min id)) + (params (request-parameters request)) + (border-high-time (assq-ref params 'border-high-time)) + (border-low-time (assq-ref params 'border-low-time)) + (border-high-id (assq-ref params 'border-high-id)) + (border-low-id (assq-ref params 'border-low-id))) + (html-page + "Evaluation" + (build-eval-table + (handle-builds-request `((evaluation . ,id) + (nr . ,%page-size) + (order . finish-time+build-id) + (border-high-time . ,border-high-time) + (border-low-time . ,border-low-time) + (border-high-id . ,border-high-id) + (border-low-id . ,border-low-id))) + builds-id-min + builds-id-max))))) (("static" path ...) (respond-static-file path)) ('method-not-allowed ;; 405 "Method Not Allowed" - (values (build-response #:code 405) #f db-channel)) + (values (build-response #:code 405) #f #f)) (_ (respond-not-found (uri->string (request-uri request)))))) -(define* (run-cuirass-server db #:key (host "localhost") (port 8080)) +(define* (run-cuirass-server #:key (host "localhost") (port 8080)) (let* ((host-info (gethostbyname host)) (address (inet-ntop (hostent:addrtype host-info) - (car (hostent:addr-list host-info)))) - - ;; Spawn a fiber to process database queries sequentially. We need - ;; this because guile-sqlite3 handles are not thread-safe (caching in - ;; particular), and creating one new handle for each request would be - ;; costly and may defeat statement caching. - (db-channel (make-critical-section db))) + (car (hostent:addr-list host-info))))) (log-message "listening on ~A:~A" address port) ;; Here we use our own web backend, call 'fiberized'. We cannot use the @@ -371,7 +351,7 @@ (spawn-fiber (lambda () (let-values (((response body state) - (handle-request (cut url-handler <> <> db-channel) + (handle-request (cut url-handler <> <>) request body '()))) (write-client impl server client response body))))) (loop))))) diff --git a/src/cuirass/utils.scm b/src/cuirass/utils.scm index 6083890..48e797c 100644 --- a/src/cuirass/utils.scm +++ b/src/cuirass/utils.scm @@ -103,17 +103,18 @@ then be passed to 'join-critical-section', which will ensure sequential ordering. ARGS are the arguments of the critical section. Critical sections are implemented by passing the procedure to execute to a -dedicated fiber." - (let ((channel (make-channel))) - (spawn-fiber - (lambda () - (parameterize ((%critical-section-args args)) - (let loop () - (match (get-message channel) - (((? channel? reply) . (? procedure? proc)) - (put-message reply (apply proc args)))) - (loop))))) - channel)) +dedicated thread." + (parameterize (((@@ (fibers internal) current-fiber) #f)) + (let ((channel (make-channel))) + (call-with-new-thread + (lambda () + (parameterize ((%critical-section-args args)) + (let loop () + (match (get-message channel) + (((? channel? reply) . (? procedure? proc)) + (put-message reply (apply proc args)))) + (loop))))) + channel))) (define (call-with-critical-section channel proc) "Send PROC to the critical section through CHANNEL. Return the result of diff --git a/tests/database.scm b/tests/database.scm index af518bd..cdc7872 100644 --- a/tests/database.scm +++ b/tests/database.scm @@ -21,6 +21,7 @@ (use-modules (cuirass database) ((guix utils) #:select (call-with-temporary-output-file)) + (cuirass utils) (srfi srfi-64)) (define example-spec @@ -61,12 +62,12 @@ (#:log . "log") (#:outputs . (("foo" . "/foo"))))) -(define-syntax-rule (with-temporary-database db body ...) +(define-syntax-rule (with-temporary-database body ...) (call-with-temporary-output-file (lambda (file port) (parameterize ((%package-database file)) (db-init file) - (with-database db + (with-database body ...))))) (define %db @@ -79,7 +80,10 @@ (test-group-with-cleanup "database" (test-assert "db-init" - (%db (db-init database-name))) + (begin + (%db (db-init database-name)) + (%db-channel (make-critical-section (%db))) + #t)) (test-assert "sqlite-exec" (begin @@ -94,41 +98,40 @@ INSERT INTO Evaluations (specification, commits) VALUES (3, 3);") (test-equal "db-add-specification" example-spec (begin - (db-add-specification (%db) example-spec) - (car (db-get-specifications (%db))))) + (db-add-specification example-spec) + (car (db-get-specifications)))) (test-equal "db-add-build" #f (let ((build (make-dummy-build "/foo.drv"))) - (db-add-build (%db) build) + (db-add-build build) ;; Should return #f when adding a build whose derivation is already ;; there, see . - (db-add-build (%db) build))) + (db-add-build build))) (test-equal "db-update-build-status!" (list (build-status scheduled) (build-status started) (build-status succeeded) "/foo.drv.log") - (with-temporary-database db + (with-temporary-database (let* ((derivation (db-add-build - db (make-dummy-build "/foo.drv" 1 #:outputs '(("out" . "/foo"))))) (get-status (lambda* (#:optional (key #:status)) - (assq-ref (db-get-build db derivation) key)))) - (db-add-evaluation db (make-dummy-eval)) - (db-add-specification db example-spec) + (assq-ref (db-get-build derivation) key)))) + (db-add-evaluation (make-dummy-eval)) + (db-add-specification example-spec) (let ((status0 (get-status))) - (db-update-build-status! db "/foo.drv" (build-status started)) + (db-update-build-status! "/foo.drv" (build-status started)) (let ((status1 (get-status))) - (db-update-build-status! db "/foo.drv" (build-status succeeded) + (db-update-build-status! "/foo.drv" (build-status succeeded) #:log-file "/foo.drv.log") ;; Second call shouldn't make any difference. - (db-update-build-status! db "/foo.drv" (build-status succeeded) + (db-update-build-status! "/foo.drv" (build-status succeeded) #:log-file "/foo.drv.log") (let ((status2 (get-status)) @@ -144,61 +147,61 @@ INSERT INTO Evaluations (specification, commits) VALUES (3, 3);") ((3 "/baz.drv") (2 "/bar.drv") (1 "/foo.drv")) ;ditto ((3 "/baz.drv")) ;nr = 1 ((2 "/bar.drv") (1 "/foo.drv") (3 "/baz.drv"))) ;status+submission-time - (with-temporary-database db + (with-temporary-database ;; Populate the 'Builds'', 'Evaluations', and ;; 'Specifications' tables in a consistent way, as expected by the ;; 'db-get-builds' query. - (db-add-build db (make-dummy-build "/foo.drv" 1 - #:outputs `(("out" . "/foo")))) - (db-add-build db (make-dummy-build "/bar.drv" 2 - #:outputs `(("out" . "/bar")))) - (db-add-build db (make-dummy-build "/baz.drv" 3 - #:outputs `(("out" . "/baz")))) - (db-add-evaluation db (make-dummy-eval)) - (db-add-evaluation db (make-dummy-eval)) - (db-add-evaluation db (make-dummy-eval)) - (db-add-specification db example-spec) - - (db-update-build-status! db "/bar.drv" (build-status started) + (db-add-build (make-dummy-build "/foo.drv" 1 + #:outputs `(("out" . "/foo")))) + (db-add-build (make-dummy-build "/bar.drv" 2 + #:outputs `(("out" . "/bar")))) + (db-add-build (make-dummy-build "/baz.drv" 3 + #:outputs `(("out" . "/baz")))) + (db-add-evaluation (make-dummy-eval)) + (db-add-evaluation (make-dummy-eval)) + (db-add-evaluation (make-dummy-eval)) + (db-add-specification example-spec) + + (db-update-build-status! "/bar.drv" (build-status started) #:log-file "/bar.drv.log") (let ((summarize (lambda (alist) (list (assq-ref alist #:id) (assq-ref alist #:derivation))))) - (vector (map summarize (db-get-builds db '((nr . 3) - (order . build-id)))) - (map summarize (db-get-builds db '())) - (map summarize (db-get-builds db '((jobset . "guix")))) - (map summarize (db-get-builds db '((nr . 1)))) + (vector (map summarize (db-get-builds '((nr . 3) (order . build-id)))) + (map summarize (db-get-builds '())) + (map summarize (db-get-builds '((jobset . "guix")))) + (map summarize (db-get-builds '((nr . 1)))) (map summarize - (db-get-builds - db '((order . status+submission-time)))))))) + (db-get-builds '((order . status+submission-time)))))))) (test-equal "db-get-pending-derivations" '("/bar.drv" "/foo.drv") - (with-temporary-database db + (with-temporary-database ;; Populate the 'Builds', 'Evaluations', and ;; 'Specifications' tables. Here, two builds map to the same derivation ;; but the result of 'db-get-pending-derivations' must not contain any ;; duplicate. - (db-add-build db (make-dummy-build "/foo.drv" 1 - #:outputs `(("out" . "/foo")))) - (db-add-build db (make-dummy-build "/bar.drv" 2 - #:outputs `(("out" . "/bar")))) - (db-add-build db (make-dummy-build "/foo.drv" 3 - #:outputs `(("out" . "/foo")))) - (db-add-evaluation db (make-dummy-eval)) - (db-add-evaluation db (make-dummy-eval)) - (db-add-evaluation db (make-dummy-eval)) - (db-add-specification db example-spec) - - (sort (db-get-pending-derivations db) stringscm))) (test-assert "db-init" - (%db (db-init database-name))) + (begin + (%db (db-init database-name)) + (%db-channel (make-critical-section (%db))) + #t)) (test-assert "cuirass-run" (call-with-new-thread (lambda () (run-fibers (lambda () - (run-cuirass-server (%db) #:port 6688)) + (run-cuirass-server #:port 6688)) #:drain? #t)))) (test-assert "wait-server" @@ -184,11 +187,11 @@ (evaluation2 '((#:specification . "guix") (#:commits . ("fakesha2" "fakesha3"))))) - (db-add-build (%db) build1) - (db-add-build (%db) build2) - (db-add-specification (%db) specification) - (db-add-evaluation (%db) evaluation1) - (db-add-evaluation (%db) evaluation2))) + (db-add-build build1) + (db-add-build build2) + (db-add-specification specification) + (db-add-evaluation evaluation1) + (db-add-evaluation evaluation2))) (test-assert "/build/1" (hash-table=? @@ -275,4 +278,6 @@ (test-assert "db-close" (db-close (%db))) - (delete-file database-name)) + (begin + (%db-channel #f) + (delete-file database-name))) -- 2.18.0 From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Resent-From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 06 Aug 2018 19:36:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153358411223973 (code B ref 32234); Mon, 06 Aug 2018 19:36:02 +0000 Received: (at 32234) by debbugs.gnu.org; 6 Aug 2018 19:35:12 +0000 Received: from localhost ([127.0.0.1]:42911 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fmlHc-0006Eb-9K for submit@debbugs.gnu.org; Mon, 06 Aug 2018 15:35:12 -0400 Received: from mail.lassieur.org ([83.152.10.219]:33010) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fmlHZ-0006ES-JC for 32234@debbugs.gnu.org; Mon, 06 Aug 2018 15:35:10 -0400 Received: from rodion (88.191.118.83 [88.191.118.83]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id a395fe21 (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO) for <32234@debbugs.gnu.org>; Mon, 6 Aug 2018 19:35:08 +0000 (UTC) References: <20180806192736.1747-1-clement@lassieur.org> <20180806192736.1747-2-clement@lassieur.org> User-agent: mu4e 1.0; emacs 26.1 From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <20180806192736.1747-2-clement@lassieur.org> Date: Mon, 06 Aug 2018 21:35:08 +0200 Message-ID: <87r2jbmgpf.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Hi, Cl=C3=A9ment Lassieur writes: [...] > bin/cuirass.in | 23 +- > src/cuirass/base.scm | 94 +++---- > src/cuirass/database.scm | 550 +++++++++++++++++++++------------------ > src/cuirass/http.scm | 136 +++++----- > src/cuirass/utils.scm | 23 +- > tests/database.scm | 103 ++++---- > tests/http.scm | 21 +- > 7 files changed, 494 insertions(+), 456 deletions(-) Pros: - fix the fact that long SQL queries block the scheduler - simplify the code, and make it more uniform Cons: - we don't take advantage of the SQLite multi-threading features anymore I've run some tests, and I didn't see any difference in terms of performance. Cl=C3=A9ment From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Resent-From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Sun, 19 Aug 2018 14:07:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153468757631293 (code B ref 32234); Sun, 19 Aug 2018 14:07:02 +0000 Received: (at 32234) by debbugs.gnu.org; 19 Aug 2018 14:06:16 +0000 Received: from localhost ([127.0.0.1]:54718 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1frOLP-00088f-Rt for submit@debbugs.gnu.org; Sun, 19 Aug 2018 10:06:16 -0400 Received: from eggs.gnu.org ([208.118.235.92]:50000) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1frOLL-00088N-Er for 32234@debbugs.gnu.org; Sun, 19 Aug 2018 10:06:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1frOLF-0004SH-3x for 32234@debbugs.gnu.org; Sun, 19 Aug 2018 10:06:06 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50 autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:50267) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1frOLF-0004SD-1C; Sun, 19 Aug 2018 10:06:05 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=50728 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1frOLE-0002Lu-Ni; Sun, 19 Aug 2018 10:06:04 -0400 From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) References: <20180806192736.1747-1-clement@lassieur.org> <20180806192736.1747-2-clement@lassieur.org> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 2 Fructidor an 226 de la =?UTF-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Sun, 19 Aug 2018 16:06:03 +0200 In-Reply-To: <20180806192736.1747-2-clement@lassieur.org> ("=?UTF-8?Q?Cl=C3=A9ment?= Lassieur"'s message of "Mon, 6 Aug 2018 21:27:36 +0200") Message-ID: <87zhxih2no.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -5.0 (-----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -6.0 (------) Hi Cl=C3=A9ment! Cl=C3=A9ment Lassieur skribis: > Fixes . > > * bin/cuirass.in (main): Keep only one WITH-DATABASE call around all fibe= rs. > Remove all DB arguments. > * src/cuirass/base.scm (evaluate, update-build-statuses!, spawn-builds, > handle-build-event, build-packages): Remove all DB arguments. > (clear-build-queue, cancel-old-builds): Wrap in WITH-DB-CRITICAL-SECTION, > remove all DB arguments. > (restart-builds): Remove the NON-BLOCKING call, remove all DB arguments. > (process-specs): Remove all DB arguments, remove the WITH-DATABASE call. > * src/cuirass/database.scm (%db-channel): New parameter. > (with-db-critical-section): New macro. > (db-add-input, db-add-specification, db-get-inputs, db-get-specifications, > db-add-evaluation, db-add-build, db-update-build-status!, db-get-outputs, > db-get-builds, db-get-build, db-get-pending-derivations, db-get-stamp, > db-add-stamp, db-get-evaluations, db-get-evaluations-build-summary, > db-get-evaluations-id-min, db-get-evaluations-id-max, db-get-builds-min, > db-get-builds-max): Wrap in WITH-DB-CRITICAL-SECTION, remove all DB argum= ents. > (with-database): Wrap BODY in PARAMETERIZE form that sets %DB-CHANNEL to = the > channel returned by MAKE-CRITICAL-SECTION. > * src/cuirass/http.scm (handle-build-request, handle-builds-request): Rem= ove > all DB arguments. > (url-handler): Remove all DB arguments, remove the DB-CHANNEL state, remo= ve > the WITH-CRITICAL-SECTION calls. > (run-cuirass-server): Remove the DB arguments, remove the > MAKE-CRITICAL-SECTION call. > * src/cuirass/utils.scm (make-critical-section): Replace SPAWN-FIBER with > CALL-WITH-NEW-THREAD. Wrap body in PARAMETERIZE form that clears > CURRENT-FIBER. > * tests/database.scm (with-temporary-database, db-add-specification, > db-add-build, db-update-build-status!, db-get-builds, > db-get-pending-derivations): Remove the DB arguments. > (db-init): Set the %DB-CHANNEL parameter to the channel returned by > MAKE-CRITICAL-SECTION, and return #t. > (database): Set %DB-CHANNEL to #f during cleanup. > * tests/http.scm (db-init): Set the %DB-CHANNEL parameter to the channel > returned by MAKE-CRITICAL-SECTION, and return #t. > (cuirass-run, fill-db): Remove the DB arguments. > (http): Set %DB-CHANNEL to #f during cleanup. Excellent, thanks for working on this! This looks great to me, and I think the pros outweigh the cons. Did you check on a big database how well it performs? One comment: > -(define* (handle-build-event db event) > +(define* (handle-build-event event) > "Handle EVENT, a build event sexp as produced by 'build-event-output-p= ort', > -updating DB accordingly." > +updating the database accordingly." [...] > +(define %db-channel > + (make-parameter #f)) > + > +(define-syntax-rule (with-db-critical-section db exp ...) > + "Evaluate EXP... in the critical section corresponding to %DB-CHANNEL. > +DB is bound to the argument of that critical section: the database > +connection." > + (call-with-critical-section (%db-channel) > + (lambda (db) exp ...))) > + I think I find it nicer to keep the =E2=80=98db=E2=80=99 parameter everywhe= re (except that it=E2=80=99s now a channel instead of an actual database) rather than = using this global variable. WDYT? Really happy we have a solution to this problem! Thank you, Ludo=E2=80=99. From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Resent-From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Sun, 26 Aug 2018 14:08:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153529243832059 (code B ref 32234); Sun, 26 Aug 2018 14:08:01 +0000 Received: (at 32234) by debbugs.gnu.org; 26 Aug 2018 14:07:18 +0000 Received: from localhost ([127.0.0.1]:33192 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ftvhE-0008Kz-HQ for submit@debbugs.gnu.org; Sun, 26 Aug 2018 10:07:18 -0400 Received: from mail.lassieur.org ([83.152.10.219]:52986) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ftvhC-0008Kr-O2 for 32234@debbugs.gnu.org; Sun, 26 Aug 2018 10:07:15 -0400 Received: from rodion (88.191.118.83 [88.191.118.83]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id 9ea3f305 (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO); Sun, 26 Aug 2018 14:05:35 +0000 (UTC) References: <20180806192736.1747-1-clement@lassieur.org> <20180806192736.1747-2-clement@lassieur.org> <87zhxih2no.fsf@gnu.org> User-agent: mu4e 1.0; emacs 26.1 From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <87zhxih2no.fsf@gnu.org> Date: Sun, 26 Aug 2018 16:07:12 +0200 Message-ID: <87lg8tb4rz.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Hi Ludovic, Ludovic Court=C3=A8s writes: > Excellent, thanks for working on this! This looks great to me, and I > think the pros outweigh the cons. Did you check on a big database how > well it performs? Yes, I didn't see any difference. When I use Berlin's database, it works well but crashes quickly for another reason (lack of disk space I think, and /tmp being tmpfs). > One comment: > >> -(define* (handle-build-event db event) >> +(define* (handle-build-event event) >> "Handle EVENT, a build event sexp as produced by 'build-event-output-= port', >> -updating DB accordingly." >> +updating the database accordingly." > > [...] > >> +(define %db-channel >> + (make-parameter #f)) >> + >> +(define-syntax-rule (with-db-critical-section db exp ...) >> + "Evaluate EXP... in the critical section corresponding to %DB-CHANNEL. >> +DB is bound to the argument of that critical section: the database >> +connection." >> + (call-with-critical-section (%db-channel) >> + (lambda (db) exp ...))) >> + > > I think I find it nicer to keep the =E2=80=98db=E2=80=99 parameter everyw= here (except > that it=E2=80=99s now a channel instead of an actual database) rather tha= n using > this global variable. > > WDYT? That 'db' parameter made sense before, because there were different database connections: one per fiber. But now that there is only one global channel accessible from everywhere, I can't find any use for a 'db-channel' parameter. Also, using two differents channels for the same database would be a bug, it would break the serialization mechanism. And I don't think using several databases (with one channel per database) would make sense either. WDYT? From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Resent-From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Sun, 26 Aug 2018 21:17:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.15353181938064 (code B ref 32234); Sun, 26 Aug 2018 21:17:01 +0000 Received: (at 32234) by debbugs.gnu.org; 26 Aug 2018 21:16:33 +0000 Received: from localhost ([127.0.0.1]:33466 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fu2Oe-00025z-RG for submit@debbugs.gnu.org; Sun, 26 Aug 2018 17:16:33 -0400 Received: from eggs.gnu.org ([208.118.235.92]:45091) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fu2Od-00025m-2l for 32234@debbugs.gnu.org; Sun, 26 Aug 2018 17:16:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fu2OW-0005SW-W0 for 32234@debbugs.gnu.org; Sun, 26 Aug 2018 17:16:25 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50 autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:51925) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fu2OW-0005SS-SM; Sun, 26 Aug 2018 17:16:24 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=50022 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1fu2OW-0003iO-KA; Sun, 26 Aug 2018 17:16:24 -0400 From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) References: <20180806192736.1747-1-clement@lassieur.org> <20180806192736.1747-2-clement@lassieur.org> <87zhxih2no.fsf@gnu.org> <87lg8tb4rz.fsf@lassieur.org> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 9 Fructidor an 226 de la =?UTF-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Sun, 26 Aug 2018 23:16:23 +0200 In-Reply-To: <87lg8tb4rz.fsf@lassieur.org> ("=?UTF-8?Q?Cl=C3=A9ment?= Lassieur"'s message of "Sun, 26 Aug 2018 16:07:12 +0200") Message-ID: <87va7w6d7c.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -5.0 (-----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -6.0 (------) Hi Cl=C3=A9ment, Cl=C3=A9ment Lassieur skribis: > Ludovic Court=C3=A8s writes: > >> Excellent, thanks for working on this! This looks great to me, and I >> think the pros outweigh the cons. Did you check on a big database how >> well it performs? > > Yes, I didn't see any difference. When I use Berlin's database, it > works well but crashes quickly for another reason (lack of disk space I > think, and /tmp being tmpfs). Sounds good (not that it crashes, but that you didn=E2=80=99t see any difference ;-)). >> I think I find it nicer to keep the =E2=80=98db=E2=80=99 parameter every= where (except >> that it=E2=80=99s now a channel instead of an actual database) rather th= an using >> this global variable. >> >> WDYT? > > That 'db' parameter made sense before, because there were different > database connections: one per fiber. But now that there is only one > global channel accessible from everywhere, I can't find any use for a > 'db-channel' parameter. > > Also, using two differents channels for the same database would be a > bug, it would break the serialization mechanism. > > And I don't think using several databases (with one channel per > database) would make sense either. These are all good points, indeed. I=E2=80=99m mildly reluctant to the glo= bal parameter, but if you prefer it that way, I don=E2=80=99t mind; the end res= ult matters more than this tiny issue anyway! So: LGTM. Thank you! Ludo=E2=80=99. From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Resent-From: Danny Milosavljevic Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 27 Aug 2018 12:42:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: Ludovic =?UTF-8?Q?Court=C3=A8s?= , 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153537371613838 (code B ref 32234); Mon, 27 Aug 2018 12:42:01 +0000 Received: (at 32234) by debbugs.gnu.org; 27 Aug 2018 12:41:56 +0000 Received: from localhost ([127.0.0.1]:33666 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fuGqC-0003b8-HS for submit@debbugs.gnu.org; Mon, 27 Aug 2018 08:41:56 -0400 Received: from dd26836.kasserver.com ([85.13.145.193]:52652) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fuGqA-0003b0-Py for 32234@debbugs.gnu.org; Mon, 27 Aug 2018 08:41:55 -0400 Received: from localhost (212095005173.public.telering.at [212.95.5.173]) by dd26836.kasserver.com (Postfix) with ESMTPSA id 1EFC333602B7; Mon, 27 Aug 2018 14:41:53 +0200 (CEST) Date: Mon, 27 Aug 2018 14:41:48 +0200 From: Danny Milosavljevic Message-ID: <20180827144120.13c76371@scratchpost.org> In-Reply-To: <87lg8tb4rz.fsf@lassieur.org> References: <20180806192736.1747-1-clement@lassieur.org> <20180806192736.1747-2-clement@lassieur.org> <87zhxih2no.fsf@gnu.org> <87lg8tb4rz.fsf@lassieur.org> X-Mailer: Claws Mail 3.16.0 (GTK+ 2.24.31; x86_64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; boundary="Sig_//DUe=SY8HXh5Idu5OJ1R1+1"; protocol="application/pgp-signature" X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) --Sig_//DUe=SY8HXh5Idu5OJ1R1+1 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Cl=C3=A9ment, in the future I plan on making the actual bin/evaluate use another database= connection in order for the web interface to be isolated while it's querying. Otherwise - as it is now in master - it can happen that while you are query= ing one page, half of the things have different values than you requested - which i= s really weird. For example right now you could query for a row with status=3D42 and get ba= ck data with status=3D43 (because it has been changed in the mean time). It's better to have serializable transaction isolation to prevent this. Th= at means that each connection will have its own worldview that is fixed at the begin= ning of the connection's transaction. The worldview will update only once a new tr= ansaction starts. Therefore, it would be good for writers to have their own connection in the= long run (really, for the readers to have their own connection - but that comes out = the same). So it would be good to keep this in mind. --Sig_//DUe=SY8HXh5Idu5OJ1R1+1 Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAluD8YwACgkQ5xo1VCww uqXX/QgAqCMZppz63XdQ6aeYB0oAjKgqBbwgOHJLUrKN802OcoWMdlZtLlt/8qnq kb+z4OtTu2YkQW2jvMxFz6AsmpzLQpxklBcJSxb6p1orpXprtULwGPN2TEIaCEPH dwehWX99xlJIdvI4yRndD7ypSFJLPcrZGfzNsC8lATkD2GZVApxRe2RMsJ+BdUCL 9JMXoPYuw1kEW1FWzPjQ9BH68+nYYx5GiL+iVWlE2OY5giz48JXJhOyti7CGVDzJ MDCUPmrTFswOe4CRICS6qWOMyLtGYte1gSeSPngGLaR1BdSLRLq3fDbLIqlAqE84 uFg0NDnuf10YRwnkL9Z+aDgXGOS/lw== =niAZ -----END PGP SIGNATURE----- --Sig_//DUe=SY8HXh5Idu5OJ1R1+1-- From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Resent-From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 27 Aug 2018 13:19:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: Danny Milosavljevic Cc: Ludovic =?UTF-8?Q?Court=C3=A8s?= , 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153537590117184 (code B ref 32234); Mon, 27 Aug 2018 13:19:02 +0000 Received: (at 32234) by debbugs.gnu.org; 27 Aug 2018 13:18:21 +0000 Received: from localhost ([127.0.0.1]:33682 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fuHPR-0004T6-CJ for submit@debbugs.gnu.org; Mon, 27 Aug 2018 09:18:21 -0400 Received: from mail.lassieur.org ([83.152.10.219]:53078) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fuHPP-0004Su-N1 for 32234@debbugs.gnu.org; Mon, 27 Aug 2018 09:18:20 -0400 Received: from rodion (LFbn-1-3186-166.w90-79.abo.wanadoo.fr [90.79.195.166]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id e18e99fe (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO); Mon, 27 Aug 2018 13:16:30 +0000 (UTC) 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> User-agent: mu4e 1.0; emacs 26.1 From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <20180827144120.13c76371@scratchpost.org> Date: Mon, 27 Aug 2018 15:18:09 +0200 Message-ID: <87a7p8aqy6.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Hi Danny, Danny Milosavljevic writes: > Hi Cl=C3=A9ment, > > in the future I plan on making the actual bin/evaluate use another databa= se connection > in order for the web interface to be isolated while it's querying. I don't understand... bin/evaluate doesn't query the database at all. I don't know why it would need to. > Otherwise - as it is now in master - it can happen that while you are que= rying one > page, half of the things have different values than you requested - which= is really > weird. > > For example right now you could query for a row with status=3D42 and get = back data with > status=3D43 (because it has been changed in the mean time). Could you please show an example that I can reproduce? I don't understand what you mean. > It's better to have serializable transaction isolation to prevent this. = That means > that each connection will have its own worldview that is fixed at the beg= inning of > the connection's transaction. The worldview will update only once a new = transaction > starts. 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. I don't understand why using a different connection would improve things. > Therefore, it would be good for writers to have their own connection in t= he long run > (really, for the readers to have their own connection - but that comes ou= t the same). 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? Cl=C3=A9ment From unknown Fri Sep 05 20:55:36 2025 MIME-Version: 1.0 X-Mailer: MIME-tools 5.505 (Entity 5.505) X-Loop: help-debbugs@gnu.org From: help-debbugs@gnu.org (GNU bug Tracking System) To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Subject: bug#32234: closed (Re: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread.) Message-ID: References: <878t4rc45s.fsf@lassieur.org> <87k1ponc62.fsf@lassieur.org> X-Gnu-PR-Message: they-closed 32234 X-Gnu-PR-Package: guix Reply-To: 32234@debbugs.gnu.org Date: Mon, 27 Aug 2018 13:48:03 +0000 Content-Type: multipart/mixed; boundary="----------=_1535377683-20026-1" This is a multi-part message in MIME format... ------------=_1535377683-20026-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Your bug report #32234: Cuirass: The SQLite built in busy handler might block the Fibers sc= heduler which was filed against the guix package, has been closed. The explanation is attached below, along with your original report. If you require more details, please reply to 32234@debbugs.gnu.org. --=20 32234: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D32234 GNU Bug Tracking System Contact help-debbugs@gnu.org with problems ------------=_1535377683-20026-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at 32234-done) by debbugs.gnu.org; 27 Aug 2018 13:47:36 +0000 Received: from localhost ([127.0.0.1]:33687 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fuHrj-0005CG-P0 for submit@debbugs.gnu.org; Mon, 27 Aug 2018 09:47:35 -0400 Received: from mail.lassieur.org ([83.152.10.219]:53086) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fuHri-0005C7-Bj for 32234-done@debbugs.gnu.org; Mon, 27 Aug 2018 09:47:34 -0400 Received: from rodion (LFbn-1-3186-166.w90-79.abo.wanadoo.fr [90.79.195.166]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id 85d2b024 (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO); Mon, 27 Aug 2018 13:45:44 +0000 (UTC) References: <20180806192736.1747-1-clement@lassieur.org> <20180806192736.1747-2-clement@lassieur.org> <87zhxih2no.fsf@gnu.org> <87lg8tb4rz.fsf@lassieur.org> <87va7w6d7c.fsf@gnu.org> User-agent: mu4e 1.0; emacs 26.1 From: =?utf-8?Q?Cl=C3=A9ment?= Lassieur To: Ludovic =?utf-8?Q?Court=C3=A8s?= Subject: Re: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. In-reply-to: <87va7w6d7c.fsf@gnu.org> Date: Mon, 27 Aug 2018 15:47:27 +0200 Message-ID: <878t4rc45s.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 32234-done Cc: 32234-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Ludovic Court=C3=A8s writes: > Hi Cl=C3=A9ment, > > Cl=C3=A9ment Lassieur skribis: > >> Ludovic Court=C3=A8s writes: >> >>> Excellent, thanks for working on this! This looks great to me, and I >>> think the pros outweigh the cons. Did you check on a big database how >>> well it performs? >> >> Yes, I didn't see any difference. When I use Berlin's database, it >> works well but crashes quickly for another reason (lack of disk space I >> think, and /tmp being tmpfs). > > Sounds good (not that it crashes, but that you didn=E2=80=99t see any > difference ;-)). > >>> I think I find it nicer to keep the =E2=80=98db=E2=80=99 parameter ever= ywhere (except >>> that it=E2=80=99s now a channel instead of an actual database) rather t= han using >>> this global variable. >>> >>> WDYT? >> >> That 'db' parameter made sense before, because there were different >> database connections: one per fiber. But now that there is only one >> global channel accessible from everywhere, I can't find any use for a >> 'db-channel' parameter. >> >> Also, using two differents channels for the same database would be a >> bug, it would break the serialization mechanism. >> >> And I don't think using several databases (with one channel per >> database) would make sense either. > > These are all good points, indeed. I=E2=80=99m mildly reluctant to the g= lobal > parameter, but if you prefer it that way, I don=E2=80=99t mind; the end r= esult > matters more than this tiny issue anyway! > > So: LGTM. > > Thank you! > > Ludo=E2=80=99. Ok, pushed! ------------=_1535377683-20026-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at submit) by debbugs.gnu.org; 21 Jul 2018 09:58:21 +0000 Received: from localhost ([127.0.0.1]:50219 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fgoeX-0002hW-EH for submit@debbugs.gnu.org; Sat, 21 Jul 2018 05:58:21 -0400 Received: from eggs.gnu.org ([208.118.235.92]:43839) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fgoeV-0002hJ-8F for submit@debbugs.gnu.org; Sat, 21 Jul 2018 05:58:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fgoeP-0001f4-3o for submit@debbugs.gnu.org; Sat, 21 Jul 2018 05:58:09 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50 autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:56937) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fgoeP-0001ex-0t for submit@debbugs.gnu.org; Sat, 21 Jul 2018 05:58:09 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34810) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fgoeN-0001if-Hr for bug-guix@gnu.org; Sat, 21 Jul 2018 05:58:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fgoeK-0001e6-HP for bug-guix@gnu.org; Sat, 21 Jul 2018 05:58:07 -0400 Received: from mail.lassieur.org ([83.152.10.219]:60320) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fgoeK-0001dS-57 for bug-guix@gnu.org; Sat, 21 Jul 2018 05:58:04 -0400 Received: from rodion (88.191.118.83 [88.191.118.83]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id 94315a1b (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO) for ; Sat, 21 Jul 2018 09:57:58 +0000 (UTC) User-agent: mu4e 1.0; emacs 26.1 From: =?utf-8?Q?Cl=C3=A9ment?= Lassieur To: bug-guix@gnu.org Subject: Cuirass: The SQLite built in busy handler might block the Fibers scheduler Date: Sat, 21 Jul 2018 11:57:57 +0200 Message-ID: <87k1ponc62.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -4.1 (----) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -5.1 (-----) Hi, I'm trying to understand why Berlin web API times out[1]. I think the SQlite built in busy handler may block the Fibers scheduler. We use "PRAGMA busy_timeout =3D 30000;", which is an alternative to calling sqlite3_busy_timeout(), whose description[2] is: This routine sets a busy handler that sleeps for a specified amount of time when a table is locked. The handler will sleep multiple times until at least "ms" milliseconds of sleeping have accumulated. After at least "ms" milliseconds of sleeping, the handler returns 0 which causes sqlite3_step() to return SQLITE_BUSY. To me this sounds like non-cooperative and non-resumable code. A solution would be to set a custom handler through sqlite3_busy_handler[3] that would be Fibers compatible, i.e. it would let the scheduler schedule other fibers instead of just sleeping, using Fibers 'sleep' procedure[4]. WDYT? Cl=C3=A9ment [1]: https://bugs.gnu.org/32233. [2]: https://www.sqlite.org/c3ref/busy_timeout.html [3]: https://www.sqlite.org/c3ref/busy_handler.html [4]: https://github.com/wingo/fibers/wiki/Manual#user-content-index-sleep ------------=_1535377683-20026-1-- From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Resent-From: Danny Milosavljevic Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 27 Aug 2018 14:25:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: Ludovic =?UTF-8?Q?Court=C3=A8s?= , 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153537984324038 (code B ref 32234); Mon, 27 Aug 2018 14:25:02 +0000 Received: (at 32234) by debbugs.gnu.org; 27 Aug 2018 14:24:03 +0000 Received: from localhost ([127.0.0.1]:34387 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fuIR0-0006Fe-Lt for submit@debbugs.gnu.org; Mon, 27 Aug 2018 10:24:02 -0400 Received: from dd26836.kasserver.com ([85.13.145.193]:33722) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fuIQz-0006FB-2Q for 32234@debbugs.gnu.org; Mon, 27 Aug 2018 10:24:01 -0400 Received: from localhost (212095005173.public.telering.at [212.95.5.173]) by dd26836.kasserver.com (Postfix) with ESMTPSA id 413723360B09; Mon, 27 Aug 2018 16:23:59 +0200 (CEST) Date: Mon, 27 Aug 2018 16:23:53 +0200 From: Danny Milosavljevic Message-ID: <20180827162353.1bdeef85@scratchpost.org> In-Reply-To: <87a7p8aqy6.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> X-Mailer: Claws Mail 3.16.0 (GTK+ 2.24.31; x86_64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; boundary="Sig_/bUux25L=SetgvRg92wn2lLF"; protocol="application/pgp-signature" X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) --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-- From unknown Fri Sep 05 20:55:36 2025 X-Loop: help-debbugs@gnu.org Subject: bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. Resent-From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Mon, 27 Aug 2018 15:06:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32234 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: Danny Milosavljevic Cc: Ludovic =?UTF-8?Q?Court=C3=A8s?= , 32234@debbugs.gnu.org Received: via spool by 32234-submit@debbugs.gnu.org id=B32234.153538231727990 (code B ref 32234); Mon, 27 Aug 2018 15:06:01 +0000 Received: (at 32234) by debbugs.gnu.org; 27 Aug 2018 15:05:17 +0000 Received: from localhost ([127.0.0.1]:34406 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fuJ4v-0007HL-4M for submit@debbugs.gnu.org; Mon, 27 Aug 2018 11:05:17 -0400 Received: from mail.lassieur.org ([83.152.10.219]:53102) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fuJ4r-0007H5-HA for 32234@debbugs.gnu.org; Mon, 27 Aug 2018 11:05:14 -0400 Received: from rodion (LFbn-1-3186-166.w90-79.abo.wanadoo.fr [90.79.195.166]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id ba78b385 (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO); Mon, 27 Aug 2018 15:03:23 +0000 (UTC) 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> User-agent: mu4e 1.0; emacs 26.1 From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <20180827162353.1bdeef85@scratchpost.org> Date: Mon, 27 Aug 2018 17:05:03 +0200 Message-ID: <874lffc0kg.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) 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.