From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: bug#32190: [PATCH] database: Merge Derivations into Builds table. Date: Tue, 14 Aug 2018 21:04:23 +0200 Message-ID: <87h8jwvkg8.fsf@elephly.net> References: <87k1ptirr0.fsf@lassieur.org> <20180804160303.20451-1-clement@lassieur.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:44481) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fqcOL-0002T7-L8 for bug-guix@gnu.org; Fri, 17 Aug 2018 06:54:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fqcOI-0004un-7o for bug-guix@gnu.org; Fri, 17 Aug 2018 06:54:05 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:48255) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fqcOI-0004ug-29 for bug-guix@gnu.org; Fri, 17 Aug 2018 06:54:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fqcOH-0002cw-TN for bug-guix@gnu.org; Fri, 17 Aug 2018 06:54:01 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-reply-to: <20180804160303.20451-1-clement@lassieur.org> List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: 32190@debbugs.gnu.org Hi Cl=C3=A9ment, > Fixes . Woo! Thank you for this patch. > * src/cuirass/base.scm (evaluate): Don't add jobs to the Derivations tabl= e. I see that you=E2=80=99ve mentioned your changes to =E2=80=9Cbuild-packages= =E2=80=9D in a later email. I have two general questions about this: why was the change from =E2=80=9Ci= d=E2=80=9D to =E2=80=9Crowid=E2=80=9D necessary? And: could you please also update the d= ocumentation so that is reflects the changes? > diff --git a/src/cuirass/database.scm b/src/cuirass/database.scm > index b4b1652..7788ac9 100644 > --- a/src/cuirass/database.scm > +++ b/src/cuirass/database.scm [=E2=80=A6] > (define (db-add-evaluation db eval) > (sqlite-exec db "\ > INSERT INTO Evaluations (specification, commits) VALUES (" > @@ -384,27 +356,39 @@ string." > (define (db-add-build db build) > "Store BUILD in database DB. BUILD eventual outputs are stored > in the OUTPUTS table." > - (let* ((build-exec > - (sqlite-exec db "\ > -INSERT INTO Builds (derivation, evaluation, log, status, timestamp, star= ttime, stoptime)\ > - VALUES (" > - (assq-ref build #:derivation) ", " > - (assq-ref build #:eval-id) ", " > - (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) ");")) > - (build-id (last-insert-rowid db))) > - (for-each (lambda (output) > - (match output > - ((name . path) > - (sqlite-exec db "\ > -INSERT INTO Outputs (build, name, path) VALUES (" > - build-id ", " name ", " path ");")))) > - (assq-ref build #:outputs)) > - build-id)) > + (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 "\ > +INSERT INTO Outputs (derivation, name, path) VALUES (" > + derivation ", " name ", " path ");")= ))) > + (assq-ref build #:outputs)) > + derivation)) This procedure is called when a build is scheduled, isn=E2=80=99t it? The docstring says =E2=80=9CBUILD eventual outputs are stored in the OUTPUTS ta= ble.=E2=80=9D =E2=80=93 does this mean the names of the *expected* output directories are recorded in Outputs? They don=E2=80=99t exist at this point, right? > + (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 (=3D code SQLITE_CONSTRAINT_PRIMARYKEY) > + #f > + (apply throw key who code rest))))) Okay. Can we prevent this from happening in the first place? I feel a little uncomfortable about performing an operation that we expect to cause primary key errors regularly. > diff --git a/src/schema.sql b/src/schema.sql > index eb0f7e9..0452495 100644 > --- a/src/schema.sql > +++ b/src/schema.sql [=E2=80=A6] > --- Builds are not in a one to one relationship with derivations in order= to > --- keep track of non deterministic compilations. Is this comment still correct considering that the derivation is now the primary key of the Builds table? > CREATE TABLE Builds ( > - id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, > - derivation TEXT NOT NULL, > + derivation TEXT NOT NULL PRIMARY KEY, > evaluation INTEGER NOT NULL, > + job_name TEXT NOT NULL, > + system TEXT NOT NULL, > + nix_name TEXT NOT NULL, > log TEXT NOT NULL, > status INTEGER NOT NULL, > timestamp INTEGER NOT NULL, > starttime INTEGER NOT NULL, > stoptime INTEGER NOT NULL, > - FOREIGN KEY (derivation) REFERENCES Derivations (derivation), > FOREIGN KEY (evaluation) REFERENCES Evaluations (id) > ); Thanks again! -- Ricardo