maildirsize is recalculated too often

Teodor Milkov <[email protected]> Thu, 03 Jul 2014 18:36:38 +0300
Newsgroups gmane.mail.vpopmail
Message-ID <[email protected]>
This is a multi-part message in MIME format.
--------------030708050504000801060500
Content-Type: multipart/alternative;
 boundary="------------010706050704000600040900"


--------------010706050704000600040900
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

After upgrading to vpopmail 5.4.33 I started seeing much more IO 
requests. I see that in 5.4.32 the following code was added to 
maildirquota.c:

  /*
       Maildir++ specification says to rebuild the maildirsize file if the
       file is 5120 or more bytes, or is more than 15 minutes old
   */

  ret = fstat(f, statptr);
  if ((ret != -1) && ((statptr->st_size >= 5120) || (time(NULL) > 
statptr->st_mtime + (15*60)))) {
     unlink(filename);
     close(f);
     return -1;
  }

I think this is not correct interpretation of the specs from 
www.courier-mta.org/imap/README.maildirquota.html which says:

/If the numbers we got indicated that the Maildir++ *is over quota*, 
some additional logic is in order: if we did not recalculate 
//maildirsize//, if the numbers in //maildirsize//indicated that we are 
over quota, then if //maildirsize//was more than one line long, or if 
the timestamp on //maildirsize//indicated that it's at least 15 minutes 
old, throw out the totals, and recalculate //maildirsize//from scratch./

I.e. the 15 minute logic should be applied only if the Maildir is 
currently over quota. This logic was already implemented in 
maildirquota.c dockeckquota() where it says if (maildirsize_nlines == 1 
&& tm < stat_buf.st_mtime + 15*60), but it didn't work, because 
/stat_buf/ is not set in maildirsize_read().

Please find attached a patch against 5.4.33, which I'm using in 
production. In addition to the minimal fix (initializing stat_buf) it 
has some documentation as well as some magic numbers converted to 
constants -- feel free to use it however you like.


Best regards,
Teodor


!DSPAM:53b5788881086521514644!

--------------010706050704000600040900
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

<html>
  <head>

    <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hi,<br>
    <br>
    After upgrading to vpopmail 5.4.33 I started seeing much more IO
    requests. I see that in 5.4.32 the following code was added to
    maildirquota.c:<br>
    <tt><br>
    </tt><tt>&nbsp;/*</tt><tt><br>
    </tt><tt>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Maildir++ specification says to rebuild the
      maildirsize file if the</tt><tt><br>
    </tt><tt>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; file is 5120 or more bytes, or is more than 15
      minutes old</tt><tt><br>
    </tt><tt>&nbsp; */</tt><tt><br>
    </tt><tt><br>
    </tt><tt>&nbsp;ret = fstat(f, statptr);</tt><tt><br>
    </tt><tt>&nbsp;if ((ret != -1) &amp;&amp; ((statptr-&gt;st_size &gt;=
      5120) || (time(NULL) &gt; statptr-&gt;st_mtime + (15*60)))) {</tt><tt><br>
    </tt><tt>&nbsp;&nbsp;&nbsp; unlink(filename);</tt><tt><br>
    </tt><tt>&nbsp;&nbsp;&nbsp; close(f);</tt><tt><br>
    </tt><tt>&nbsp;&nbsp;&nbsp; return -1;</tt><tt><br>
    </tt><tt>&nbsp;}</tt><br>
    <br>
    I think this is not correct interpretation of the specs from
    <a class="moz-txt-link-abbreviated" href="http://www.courier-mta.org/imap/README.maildirquota.html">www.courier-mta.org/imap/README.maildirquota.html</a> which says:<br>
    <br>
    <i>If the numbers we got indicated that the Maildir++ <b>is over
        quota</b>, some additional logic is in order: if we did not
      recalculate </i><i><tt>maildirsize</tt></i><i>, if the numbers in
    </i><i><tt>maildirsize</tt></i><i> indicated that we are over quota,
      then if </i><i><tt>maildirsize</tt></i><i> was more than one line
      long, or if the timestamp on </i><i><tt>maildirsize</tt></i><i>
      indicated that it's at least 15 minutes old, throw out the totals,
      and recalculate </i><i><tt>maildirsize</tt></i><i> from scratch.</i><br>
    <br>
    I.e. the 15 minute logic should be applied only if the Maildir is
    currently over quota. This logic was already implemented in
    maildirquota.c dockeckquota() where it says if <tt>(maildirsize_nlines
      == 1 &amp;&amp; tm &lt; stat_buf.st_mtime + 15*60)</tt>, but it
    didn't work, because <i>stat_buf</i> is not set in
    maildirsize_read().<br>
    <br>
    Please find attached a patch against 5.4.33, which I'm using in
    production. In addition to the minimal fix (initializing stat_buf)
    it has some documentation as well as some magic numbers converted to
    constants &#8211; feel free to use it however you like.<br>
    <br>
    <br>
    Best regards,<br>
    Teodor<br>
  
!DSPAM:53b5788881086521514644!

</body>
</html>



--------------010706050704000600040900--

--------------030708050504000801060500
Content-Type: text/x-patch;
 name="vpopmail-maildirsize-recalc-specs.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="vpopmail-maildirsize-recalc-specs.patch"

diff -Nru vpopmail-5.4.33.orig/maildirquota.h vpopmail-5.4.33/maildirquota.h
--- vpopmail-5.4.33.orig/maildirquota.h	2011-02-28 19:00:45.000000000 +0200
+++ vpopmail-5.4.33/maildirquota.h	2014-07-03 18:09:48.000000000 +0300
@@ -12,6 +12,10 @@
 
 #define QUOTA_WARN_PERCENT 90
 
+/* http://www.courier-mta.org/imap/README.maildirquota.html */
+#define MAILDIRSIZE_OVERLEN 5120
+#define MAILDIRSIZE_OLD_SECS 15*60
+
 /* I've removed pretty much the whole file execept for
    some public functions so as to not conflict with courier.
    I"ve made the courier functions static.
diff -Nru vpopmail-5.4.33.orig/maildirquota.c vpopmail-5.4.33/maildirquota.c
--- vpopmail-5.4.33.orig/maildirquota.c	2011-02-28 19:00:45.000000000 +0200
+++ vpopmail-5.4.33/maildirquota.c	2014-07-03 18:14:54.000000000 +0300
@@ -333,7 +333,7 @@
 	unsigned *nlines, /* # of lines in maildirsize */
 	struct stat *statptr)	/* The stats on maildirsize */
 {
- char buf[5120];
+ char buf[MAILDIRSIZE_OVERLEN];
  int f;
  char *p;
  unsigned l;
@@ -343,21 +343,13 @@
 
 	if ((f=maildir_safeopen(filename, O_RDWR|O_APPEND, 0)) < 0)
 		return (-1);
+
+	if (fstat(f, statptr) < 0)
+		return (-1);
+
 	p=buf;
 	l=sizeof(buf);
 
-	/*
-		 Maildir++ specification says to rebuild the maildirsize file if the
-		 file is 5120 or more bytes, or is more than 15 minutes old
-    */
-
-	ret = fstat(f, statptr);
-	if ((ret != -1) && ((statptr->st_size >= 5120) || (time(NULL) > statptr->st_mtime + (15*60)))) {
-	   unlink(filename);
-	   close(f);
-	   return -1;
-    }
-
 	while (l)
 	{
 		n=read(f, p, l);
@@ -409,6 +401,17 @@
 	return (0);
 }
 
+/**
+ * Calculate quota usage in percent (%).
+ *
+ * @param[in]  s           Current maildir size in bytes.
+ * @param[in]  n           Current maildir message count.
+ * @param[in]  *quota      Current quota limit, e.g. 1000S or 1000C.
+ * @param[out] *percentage Quota usage in % (the bigger between size & count).
+ *
+ * @retval 0  within quota
+ * @retval -1 over quota
+ */
 static int qcalc(storage_t s, storage_t n, const char *quota, int *percentage)
 {
 storage_t i;
@@ -595,7 +598,7 @@
 		n=qcalc(maildirsize_size+xtra_size, maildirsize_cnt+xtra_cnt,
 			quota_type, percentage);
 
-		if (n == 0)
+		if (n == 0) // within quota
 		{
 			free(checkfolder);
 			*maildirsize_fdptr=maildirsize_fd;
@@ -603,7 +606,16 @@
 		}
 		close(maildirsize_fd);
 
-		if (maildirsize_nlines == 1 && tm < stat_buf.st_mtime + 15*60)
+		/*
+		 * http://www.courier-mta.org/imap/README.maildirquota.html
+		 *
+		 * "if the numbers in maildirsize indicated that we are over quota,
+		 * then if maildirsize was more than one line long, or if the timestamp
+		 * on maildirsize indicated that it's at least 15 minutes old, throw out
+		 * the totals, and recalculate maildirsize from scratch"
+	     */
+		if (maildirsize_nlines == 1 &&
+			tm < stat_buf.st_mtime + MAILDIRSIZE_OLD_SECS)
 			return (n);
 	}
 

--------------030708050504000801060500--