guix-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[no subject]


From: Mathieu Othacehe
Date: Thu, 11 Mar 2021 04:27:28 -0500 (EST)

branch: master
commit aabeced62ae23f148c29524dbc63d73467c3cd12
Author: Mathieu Othacehe <othacehe@gnu.org>
AuthorDate: Thu Mar 11 10:27:13 2021 +0100

    Send notifications in a separate thread.
---
 bin/cuirass.in                | 148 +++++++++++++++++++++---------------------
 src/cuirass/database.scm      |  55 +++++++++++-----
 src/cuirass/notification.scm  |  54 ++++++---------
 src/cuirass/remote-server.scm |  16 ++---
 src/schema.sql                |   6 +-
 tests/database.scm            |  61 ++++++++++++-----
 6 files changed, 187 insertions(+), 153 deletions(-)

diff --git a/bin/cuirass.in b/bin/cuirass.in
index 421b22c..65c8f98 100644
--- a/bin/cuirass.in
+++ b/bin/cuirass.in
@@ -145,80 +145,80 @@ exec ${GUILE:-@GUILE@} --no-auto-compile -e main -s "$0" 
"$@"
           (run-fibers
            (lambda ()
              (with-database
-                 (with-notification
-                  (and specfile
-                       (for-each db-add-specification
-                                 (read-specifications specfile)))
-                  (and paramfile (read-parameters paramfile))
-
-                  (if one-shot?
-                      (process-specs (db-get-specifications))
-                      (let ((exit-channel (make-channel)))
-                        (start-watchdog)
-                        (if (option-ref opts 'web #f)
-                            (begin
-                              (spawn-fiber
-                               (essential-task
-                                'web exit-channel
-                                (lambda ()
-                                  (run-cuirass-server #:host host
-                                                      #:port port)))
-                               #:parallel? #t)
-
-                              (spawn-fiber
-                               (essential-task
-                                'monitor exit-channel
-                                (lambda ()
-                                  (while #t
-                                    (log-monitoring-stats)
-                                    (sleep 600))))))
-
-                            (begin
-                              (clear-build-queue)
-
-                              ;; If Cuirass was stopped during an evaluation,
-                              ;; abort it. Builds that were not registered
-                              ;; during this evaluation will be registered
-                              ;; during the next evaluation.
-                              (db-abort-pending-evaluations)
-
-                              ;; First off, restart builds that had not
-                              ;; completed or were not even started on a
-                              ;; previous run.
-                              (spawn-fiber
-                               (essential-task
-                                'restart-builds exit-channel
-                                (lambda ()
-                                  (restart-builds))))
-
-                              (spawn-fiber
-                               (essential-task
-                                'build exit-channel
-                                (lambda ()
-                                  (while #t
-                                    (process-specs (db-get-specifications))
-                                    (log-message
-                                     "next evaluation in ~a seconds" interval)
-                                    (sleep interval)))))
-
-                              (spawn-fiber
-                               (essential-task
-                                'metrics exit-channel
-                                (lambda ()
-                                  (while #t
-                                    (with-time-logging
-                                     "Metrics update"
-                                     (db-update-metrics))
-                                    (sleep 3600)))))
-
-                              (spawn-fiber
-                               (essential-task
-                                'monitor exit-channel
-                                (lambda ()
-                                  (while #t
-                                    (log-monitoring-stats)
-                                    (sleep 600)))))))
-                        (primitive-exit (get-message exit-channel)))))))
+                 (start-notification-thread)
+                 (and specfile
+                      (for-each db-add-specification
+                                (read-specifications specfile)))
+                 (and paramfile (read-parameters paramfile))
+
+               (if one-shot?
+                   (process-specs (db-get-specifications))
+                   (let ((exit-channel (make-channel)))
+                     (start-watchdog)
+                     (if (option-ref opts 'web #f)
+                         (begin
+                           (spawn-fiber
+                            (essential-task
+                             'web exit-channel
+                             (lambda ()
+                               (run-cuirass-server #:host host
+                                                   #:port port)))
+                            #:parallel? #t)
+
+                           (spawn-fiber
+                            (essential-task
+                             'monitor exit-channel
+                             (lambda ()
+                               (while #t
+                                 (log-monitoring-stats)
+                                 (sleep 600))))))
+
+                         (begin
+                           (clear-build-queue)
+
+                           ;; If Cuirass was stopped during an evaluation,
+                           ;; abort it. Builds that were not registered
+                           ;; during this evaluation will be registered
+                           ;; during the next evaluation.
+                           (db-abort-pending-evaluations)
+
+                           ;; First off, restart builds that had not
+                           ;; completed or were not even started on a
+                           ;; previous run.
+                           (spawn-fiber
+                            (essential-task
+                             'restart-builds exit-channel
+                             (lambda ()
+                               (restart-builds))))
+
+                           (spawn-fiber
+                            (essential-task
+                             'build exit-channel
+                             (lambda ()
+                               (while #t
+                                 (process-specs (db-get-specifications))
+                                 (log-message
+                                  "next evaluation in ~a seconds" interval)
+                                 (sleep interval)))))
+
+                           (spawn-fiber
+                            (essential-task
+                             'metrics exit-channel
+                             (lambda ()
+                               (while #t
+                                 (with-time-logging
+                                  "Metrics update"
+                                  (db-update-metrics))
+                                 (sleep 3600)))))
+
+                           (spawn-fiber
+                            (essential-task
+                             'monitor exit-channel
+                             (lambda ()
+                               (while #t
+                                 (log-monitoring-stats)
+                                 (sleep 600)))))))
+                     (primitive-exit (get-message exit-channel))))))
 
            ;; Most of our code is I/O so preemption doesn't matter much (it
            ;; could help while we're doing SQL requests, for instance, but it
diff --git a/src/cuirass/database.scm b/src/cuirass/database.scm
index bba970e..edbd193 100644
--- a/src/cuirass/database.scm
+++ b/src/cuirass/database.scm
@@ -95,6 +95,8 @@
             db-get-builds-max
             db-get-evaluation-specification
             db-get-build-product-path
+            db-push-notification
+            db-pop-notification
             db-add-or-update-worker
             db-get-worker
             db-get-workers
@@ -752,29 +754,26 @@ log file for DRV."
         ;; times in a row, for instance.  The 'last_status' field is updated
         ;; with the status of the last completed build with the same
         ;; 'job_name' and 'specification'.
-        (begin
-          (let* ((last-status (db-get-last-status drv))
-                 (weather (build-status->weather status last-status))
-                 (rows
-                  (exec-query/bind db "
+        (let* ((last-status (db-get-last-status drv))
+               (weather (build-status->weather status last-status))
+               (rows
+                (exec-query/bind db "
 UPDATE Builds SET stoptime =" now
 ", status =" status
 ", last_status = " last-status
 ", weather = " weather
 "WHERE derivation =" drv
 " AND status != " status ";")))
-            (when (positive? rows)
-              (let* ((build (db-get-build drv))
-                     (spec (assq-ref build #:specification))
-                     (specification (db-get-specification spec))
-                     (notifications
-                      (specification-notifications specification)))
-                (send-notifications notifications #:build build)
-                (db-add-event 'build
-                              now
-                              `((#:derivation . ,drv)
-                                (#:event      . ,(assq-ref status-names
-                                                           status)))))))))))
+          (when (positive? rows)
+            (let* ((build (db-get-build drv))
+                   (spec (assq-ref build #:specification))
+                   (specification (db-get-specification spec))
+                   (notifications
+                    (specification-notifications specification)))
+              (for-each (lambda (notif)
+                          (db-push-notification notif
+                                                (assq-ref build #:id)))
+                        notifications)))))))
 
 (define* (db-update-build-worker! drv worker)
   "Update the database so that DRV's worker is WORKER."
@@ -1367,6 +1366,28 @@ WHERE id = " id))
       ((path) path)
       (else #f))))
 
+(define (db-push-notification notification build)
+  "Insert NOTIFICATION into Notifications table."
+  (with-db-worker-thread db
+    (exec-query/bind db "\
+INSERT INTO Notifications (type, build)
+VALUES (" (notification->sexp notification) ", " build ");")))
+
+(define (db-pop-notification)
+  "Return two values, the latest notification from the Notifications table and
+the matching build."
+  (with-db-worker-thread db
+    (match (expect-one-row
+            (exec-query/bind db "
+SELECT id, type, build from Notifications ORDER BY id ASC LIMIT 1;"))
+      ((id type build)
+       (exec-query/bind db "\
+DELETE FROM Notifications WHERE id =" id ";")
+       (cons (sexp->notification
+              (with-input-from-string type read))
+             (db-get-build (string->number build))))
+      (else #f))))
+
 (define (db-add-or-update-worker worker)
   "Insert WORKER into Worker table."
   (with-db-worker-thread db
diff --git a/src/cuirass/notification.scm b/src/cuirass/notification.scm
index 5688c77..7402467 100644
--- a/src/cuirass/notification.scm
+++ b/src/cuirass/notification.scm
@@ -17,13 +17,16 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (cuirass notification)
+  #:use-module (cuirass database)
   #:use-module (cuirass logging)
   #:use-module (cuirass mail)
   #:use-module (cuirass mastodon)
   #:use-module (cuirass parameters)
   #:use-module (cuirass utils)
+  #:use-module (guix build syscalls)
   #:use-module (guix records)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 threads)
   #:export (email
             email?
             email-from
@@ -36,8 +39,7 @@
             notification->sexp
             sexp->notification
 
-            with-notification
-            send-notifications))
+            start-notification-thread))
 
 
 ;;;
@@ -89,27 +91,6 @@
 (define weather-success 0)
 (define weather-failure 1)
 
-;; The channel to communicate with the notification worker thread.
-(define %notification-channel
-  (make-parameter #f))
-
-(define-syntax-rule (with-notification body ...)
-  "Run BODY with %NOTIFICATION-CHANNEL being dynamically bound to a channel
-providing a worker thread that allows to send notifications without
-interfering with fibers."
-  (parameterize ((%notification-channel
-                  (make-worker-thread-channel
-                   (const #t))))
-    body ...))
-
-(define-syntax-rule (with-notification-worker-thread exp ...)
-  "Evaluate EXP... in the critical section corresponding to
-%NOTIFICATION-CHANNEL."
-  (call-with-worker-thread
-   (%notification-channel)
-   (lambda args
-     exp ...)))
-
 (define (build-weather-text build)
   "Return the build weather string."
   (let ((weather (assq-ref build #:weather)))
@@ -171,14 +152,19 @@ the detailed information about this build here: ~a."
         (log-message "Failed to send the mastodon notification: ~a."
                      args)))))
 
-(define* (send-notifications notifications #:key build)
-  "Send the notifications in NOTIFICATIONS list, regarding the given BUILD."
-  (with-notification-worker-thread
-   (for-each
-    (lambda (notification)
-      (cond
-       ((email? notification)
-        (send-email* notification build))
-       ((mastodon? notification)
-        (send-mastodon build))))
-    notifications)))
+(define (start-notification-thread)
+  "Start a thread sending build notifications."
+  (call-with-new-thread
+   (lambda ()
+     (set-thread-name "notification")
+     (let loop ()
+       (match (db-pop-notification)
+         ((notif . build)
+          (cond
+           ((email? notif)
+            (send-email* notif build))
+           ((mastodon? notif)
+            (send-mastodon build))))
+         (#f #f))
+       (sleep 1)
+       (loop)))))
diff --git a/src/cuirass/remote-server.scm b/src/cuirass/remote-server.scm
index 4cd7f53..16a7e6c 100644
--- a/src/cuirass/remote-server.scm
+++ b/src/cuirass/remote-server.scm
@@ -508,11 +508,11 @@ exiting."
         (receive-logs log-port (%cache-directory))
 
         (with-database
-            (with-notification
-             (for-each (lambda (number)
-                         (start-fetch-worker
-                          (string-append "fetch-worker-"
-                                         (number->string number))))
-                       (iota 4))
-
-             (zmq-start-proxy backend-port)))))))
+            (start-notification-thread)
+            (for-each (lambda (number)
+                        (start-fetch-worker
+                         (string-append "fetch-worker-"
+                                        (number->string number))))
+                      (iota 4))
+
+            (zmq-start-proxy backend-port))))))
diff --git a/src/schema.sql b/src/schema.sql
index 90b4e6a..de5e67d 100644
--- a/src/schema.sql
+++ b/src/schema.sql
@@ -84,11 +84,11 @@ CREATE TABLE BuildProducts (
   FOREIGN KEY (build) REFERENCES Builds(id) ON DELETE CASCADE
 );
 
-CREATE TABLE Events (
+CREATE TABLE Notifications (
   id            SERIAL PRIMARY KEY,
   type          TEXT NOT NULL,
-  timestamp     INTEGER NOT NULL,
-  event_json    TEXT NOT NULL
+  build         INTEGER NOT NULL,
+  FOREIGN KEY (build) REFERENCES Builds(id) ON DELETE CASCADE
 );
 
 CREATE TABLE Workers (
diff --git a/tests/database.scm b/tests/database.scm
index a3af071..54dd7c2 100644
--- a/tests/database.scm
+++ b/tests/database.scm
@@ -103,11 +103,23 @@
    (systems '("a" "b"))
    (last-seen 1)))
 
+(define* (retry f #:key times delay)
+  (let loop ((attempt 1))
+    (let ((result (f)))
+      (cond
+       (result result)
+       (else
+        (if (>= attempt times)
+            #f
+            (begin
+              (sleep delay)
+              (loop (+ 1 attempt)))))))))
 
 (test-group-with-cleanup "database"
   (test-assert "db-init"
     (begin
       (test-init-db!)
+      (start-notification-thread)
       #t))
 
   (test-equal "db-add-specification"
@@ -231,17 +243,6 @@ timestamp, checkouttime, evaltime) VALUES ('guix', 0, 0, 
0, 0);")
     (let ((build (db-get-build "/foo.drv")))
       (assoc-ref build #:id)))
 
-  (test-equal "db-get-events"
-    'evaluation
-    (let ((event (match (db-get-events '((nr . 1)
-                                         (type . evaluation)))
-                   ((event) event))))
-      (assoc-ref event #:type)))
-
-  (test-equal "db-delete-events-with-ids-<=-to"
-    1
-    (db-delete-events-with-ids-<=-to 1))
-
   (test-equal "db-get-pending-derivations"
     '("/foo.drv")
     (db-get-pending-derivations))
@@ -492,9 +493,14 @@ timestamp, checkouttime, evaltime) VALUES ('guix', 0, 0, 
0, 0);")
       (assq-ref (db-get-build "/new-build.drv") #:weather)))
 
   (test-assert "mail notification"
-    (let ((str (call-with-input-file tmp-mail
-                 get-string-all)))
-      (string-contains str "Build job-1 on guix is fixed.")))
+    (retry
+     (lambda ()
+       (and (file-exists? tmp-mail)
+            (let ((str (call-with-input-file tmp-mail
+                         get-string-all)))
+              (string-contains str "Build job-1 on guix is fixed."))))
+     #:times 5
+     #:delay 1))
 
   (test-equal "db-get-builds weather"
     (build-weather new-failure)
@@ -504,9 +510,14 @@ timestamp, checkouttime, evaltime) VALUES ('guix', 0, 0, 
0, 0);")
       (assq-ref (db-get-build "/new-build.drv") #:weather)))
 
   (test-assert "mail notification"
-    (let ((str (call-with-input-file tmp-mail
-                 get-string-all)))
-      (string-contains str "Build job-1 on guix is broken.")))
+    (retry
+     (lambda ()
+       (and (file-exists? tmp-mail)
+            (let ((str (call-with-input-file tmp-mail
+                         get-string-all)))
+              (string-contains str "Build job-1 on guix is broken."))))
+     #:times 5
+     #:delay 1))
 
   (test-equal "db-get-builds weather"
     (build-weather still-succeeding)
@@ -548,6 +559,22 @@ timestamp, checkouttime, evaltime) VALUES ('guix', 0, 0, 
0, 0);")
       (eq? (assq-ref (db-get-build drv) #:status)
            (build-status canceled))))
 
+  (test-assert "db-push-notification"
+    (let ((build (db-get-build "/new-build.drv")))
+      (db-push-notification
+       (email
+        (from "from")
+        (to "to")
+        (server (mailer)))
+       (assq-ref build #:id))))
+
+  (test-assert "db-pop-notification"
+    (let ((build (db-get-build "/new-build.drv")))
+      (match (db-pop-notification)
+        ((notif . notif-build)
+         (and (email? notif)
+              (equal? build notif-build))))))
+
   (test-assert "db-close"
     (begin
       (db-close (%db))



reply via email to

[Prev in Thread] Current Thread [Next in Thread]