gnats/665: queue-pr -r doesn't process the incoming messages in the right order

[bug-gnats]

>Number:         665
>Category:       gnats
>Synopsis:       queue-pr -r doesn't process the incoming messages in the right 
>order
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    unassigned
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Aug 09 08:15:01 -0500 2006
>Originator:     Stephane Chazelas <address@hidden>
>Release:        
>Description:
 
 Hi guys,
 
 queue-pr --run doesn't process the queued message in the order
 they were received. They process it in the order given by
 readdir in the queue directory. That means that it may be hard
 to make some sense of the audit-trail as the messages are in
 random order.
 
 This is especially annoying when the interval between 2 queue-pr
 is long, or when you want to submit a batch of email afterwards
 for which people forgot to Cc: the submit address.
 
>Fix:

Here's a fix that implements the queue processing differently.

Incoming messages are stored into db/gnats-queue/incoming and
are named "0", "1"...

queue-pr -q, stores the message into a tempfile in
db/gnats-queue, and uses link(2) (atomic) checking the errno, to
save it as the first unused number in "incoming" (creating the
"incoming" directory if necessary).

queue-pr -r first renames "incoming" to "processing", and then
processes the messages in "processing", letting another queue-pr
-q create a new incoming directory. It also prevents two
queue-pr -r to run at the same time, as a second one aborts if
it can't rename "incoming" to "processing".

Index: gnats/queue-pr.c
===================================================================
RCS file: /sources/gnats/gnats/gnats/queue-pr.c,v
retrieving revision 1.21
diff -u -r1.21 queue-pr.c
--- gnats/queue-pr.c    20 Apr 2006 05:33:23 -0000      1.21
+++ gnats/queue-pr.c    9 Aug 2006 11:40:11 -0000
@@ -173,119 +173,93 @@
 static void
 run_gnats (DatabaseInfo database, const char *queue_dir)
 {
-  DIR *d;
-  struct dirent *next;
   int i;
-  int nfiles = 0;
-  int maxfiles = 10;
-  struct file {
-    char *name;
-  } *files = (struct file *) xmalloc (sizeof (struct file) * maxfiles);
+  char filename[32];           /* files are named 0, 1, ... */
 
   if (chdir (queue_dir) < 0)
     {
-      punt (database, 1, "can't open queue directory: %s", queue_dir);
+      punt (database, 1, "can't open %s queue directory: %s\n",
+           queue_dir, strerror (errno));
     }
 
-  errno = 0;
-  d = opendir (".");
-  if (! d)
-    {
-      log_msg (LOG_INFO, 1, "can't open: ", queue_dir);
-      return;
-    }
-
-  while ((next = readdir (d)))
-    if (next->d_name[0] != '.')
-      {
-       if (strcmp (next->d_name, "core") == 0)
-         {
-           log_msg (LOG_INFO, 0, "core file in queue directory");
-           continue;
-         }
-
-       /* skip files larger than max_size */
-       if (max_size)
-         {
-           struct stat buf;
-           if (stat ((char*) next->d_name, &buf) == 0)
-             {
-               if ((int) buf.st_size > max_size)
-                 {
-                   char *max;
-                   char *name2 = xmalloc (strlen ((char*) next->d_name) + 2);
-                   strcpy (name2, ".");
-                   strcat (name2, (char*) next->d_name);
-                   rename ((char*) next->d_name, name2);
-                   asprintf (&max, "%d", max_size / 1024);
-                   punt (database, 0,
-                         ("file `%s' larger than max-size of %sK.\n"
-                          "renamed to `%s' pending human intervention.\n"),
-                         (char*) next->d_name, max, name2);
-                   log_msg (LOG_INFO, 1, "file larger than max-size:", name2);
-                   free (name2);
-                   free (max);
-                   continue;
-                 }
-             }
-         }                 
-
-       if (nfiles == maxfiles)
-         {
-           maxfiles *= 2;
-           files = (struct file *) xrealloc ((char *) files,
-                                             sizeof (struct file) * maxfiles);
-         }
-       files[nfiles++].name = (char *) xstrdup (next->d_name);
-      }
-
-  closedir (d);
+  if (rename ("incoming", "processing") != 0)
+    {
+      switch (errno)
+       {
+       case ENOTEMPTY:
+       case EEXIST:
+         punt (database, 1, "processing of %s is already ongoing or didn't 
finish properly\n",
+               queue_dir);
+         break;
+       case ENOENT:
+         /* no message in queue */
+         return;
+       default:
+         punt (database, 1,
+               "cannot rename %s/incoming to %s/processing: %s\n", queue_dir,
+               queue_dir, strerror (errno));
+       }
+    }
+  if (chdir ("processing") < 0)
+    {
+      punt (database, 1, "can't open queue %s/processing directory: %s\n",
+           queue_dir, strerror (errno));
+    }
 
   /* Run a gnats process for each file in the directory.  */
-  for (i = 0; i < nfiles; i++)
+  for (i = 0;; i++)
     {
       int child_status;
-      
+      struct stat st;
+
       if (flag_debug)
-       fprintf (stderr, "%s: running %s\n", program_name, files[i].name);
+       fprintf (stderr, "%s: running %s\n", program_name, filename);
+
+      sprintf (filename, "%d", i);
+      if (stat (filename, &st) < 0)
+       {
+         break;
+       }
 
-      child_status = fork_gnats (database, files[i].name);
+      child_status = fork_gnats (database, filename);
 
       /* If gnats ran okay, then we can unlink the queue file.  Otherwise,
-        keep it around so we can give it another try whenever the problems
-        have been taken care of.
-        Exit status of 1: Simple failure, try again
-                       2: Unexpected failure, probably case-specific
+         keep it around so we can give it another try whenever the problems
+         have been taken care of.
+         Exit status of 1: Simple failure, try again
+                       2: Unexpected failure, probably case-specific
                        3: Unexpected failure, probably general  */
       if (child_status == 0)
        {
-         if (unlink (files[i].name))
-           log_msg (LOG_INFO, 1, "cannot remove: ", files[i].name);
+         if (unlink (filename) != 0)
+           punt(database, 0, "cannot remove queue file %s: %s\n",
+               filename, strerror(errno));
        }
       else if (child_status == 2)
        {
-         struct stat buf;
-         if (stat ((char*) files[i].name, &buf) != 0 &&
-             errno == ENOENT)
+         /* something bad has happened with this file,
+            move it aside, and alert the admin */
+         char *name2;
+         int fd;
+
+         asprintf (&name2, "%s/GNATS-ERR-%d-XXXXXX", queue_dir, i);
+         fd = open_temporary_file (name2, 0664);
+         if (fd >= 0)
            {
-             /* the file is gone, perhaps another queue-pr took care of it? */
+             close (fd);
+             rename (filename, name2);
              punt (database, 0,
-                   "file `%s' was gone when pr-edit tried to process it.\n",
-                   files[i].name);
+                   "renamed `%s' to `%s' pending human intervention.\n",
+                   filename, name2);
            }
          else
            {
-             /* something else bad has happened with this file,
-                move it aside, and alert the admin */
-             char *name2 = xmalloc (strlen (files[i].name) + 2);
-             strcpy (name2, ".");
-             strcat (name2, files[i].name);
-             rename (files[i].name, name2);
              punt (database, 0,
-                   "renamed `%s' to `%s' pending human intervention.\n",
-                   files[i].name, name2);
-             free (name2);
+                   "could not rename `%s' to `%s' for human intervention: 
%s\n",
+                   filename, name2, strerror (errno));
            }
+         free (name2);
+
          if (is_gnats_locked (database))
            {
              client_unlock_gnats ();
@@ -305,11 +279,17 @@
 drop_msg (DatabaseInfo database, const char *queue_dir)
 {
   int fd[2];
-  const char *tmpdir;
+  char tmpfile[] = "gnatsXXXXXX";
   char *bug_file;
-  int r; /* XXX ssize_t */
+  int r;                       /* XXX ssize_t */
+  int i;
   char *buf = (char *) xmalloc (MAXBSIZE);
-  char *base, *new_name;
+
+  if (chdir (queue_dir) < 0)
+    {
+      punt (database, 1, "can't open queue directory %s: %s\n",
+           queue_dir, strerror (errno));
+    }
 
   if (queue_file)
     {
@@ -321,16 +301,13 @@
   else
     fd[0] = 0;
 
-  tmpdir = temporary_directory ();
-  asprintf (&bug_file, "%s/gnatsXXXXXX", tmpdir);
-  
-  fd[1] = open_temporary_file (bug_file, 0664);
+  fd[1] = open_temporary_file (tmpfile, 0664);
   if (fd[1] < 0)
     {
       punt (database, 1, "can't open queue file %s for writing: %s\n",
-           bug_file, strerror (errno));
+           tmpfile, strerror (errno));
     }
-  
+
   while ((r = read (fd[0], buf, MAXBSIZE)) > 0)
     {
       if (write (fd[1], buf, r) < 0)
@@ -339,50 +316,62 @@
            close (fd[0]);
          close (fd[1]);
          punt (database, 1, "error in writing %s: %s\n",
-               bug_file, strerror (errno));
+               tmpfile, strerror (errno));
        }
     }
-  
+
   if (r < 0)
     {
       if (queue_file)
        punt (database, 1, "error reading queue file %s: %s\n",
              queue_file, strerror (errno));
     }
-  
+
   if (fd[0])
     {
       close (fd[0]);
     }
   close (fd[1]);
 
-  errno = 0;
-  base = basename (bug_file);
-  asprintf (&new_name, "%s/%s", queue_dir, base);
-  /* FIXME: Is there any guarantee here that NEW_NAME doesn't already exist? */
-  if (rename (bug_file, new_name) < 0)
+  for (i = 0;;)
     {
-      if (errno != EXDEV)
+      if (i == 2000)
        {
-         punt (database, 1, "could not rename %s into the queue dir: %s\n",
-               bug_file, strerror (errno));
+         punt (database, 1, "incoming queue full\n");
        }
-      
-      {
-       char *tmp_name;
-       asprintf (&tmp_name, "%s/.%s", queue_dir, base);
-       copy_file (bug_file, tmp_name);
-       if (rename (tmp_name, new_name) < 0)
-         {
-           punt (database, 1, "could not rename %s to %s: %s\n",
-                 tmp_name, new_name, strerror (errno));
-         }
-      }
+      asprintf (&bug_file, "incoming/%d", i);
 
-      if (unlink (bug_file))
+      if (link (tmpfile, bug_file) == 0)
        {
-         punt (database, 1, "cannot remove `%s'", bug_file);
+         if (unlink (tmpfile) != 0)
+           {
+             punt (database, 1, "cannot remove `%s'\n", bug_file);
+           }
+         break;
+       }
+      else
+       {
+         switch (errno)
+           {
+           case EEXIST:
+             i++;
+             break;
+           case ENOENT:
+             if (mkdir ("incoming", 0775) != 0)
+               {
+                 punt (database, 1,
+                       "cannot create the `incoming' queue dir: %s\n",
+                       strerror (errno));
+               }
+             i = 0;
+             break;
+           default:
+             punt (database, 1,
+                   "could not rename %s to the queue file %s: %s\n", tmpfile,
+                   bug_file, strerror (errno));
+           }
        }
+      free (bug_file);
     }
 }