]> granicus.if.org Git - postgresql/commitdiff
Try to ensure that stats collector's receive buffer size is at least 100KB.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 16 May 2017 19:24:52 +0000 (15:24 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 16 May 2017 19:24:52 +0000 (15:24 -0400)
Since commit 4e37b3e15, buildfarm member frogmouth has been failing
occasionally with symptoms indicating that some expected stats data is
getting dropped.  The reason that that commit changed the behavior seems
probably to be that more data is getting shoved at the collector in a short
span of time.  In current sources, the stats test's first session sends
about 9KB of data while exiting, which is probably the same as what was
sent just before wait_for_stats() in the previous test design.  But now,
the test's second session is starting up concurrently, and it sends another
2KB (presumably reflecting its initial catalog accesses).  Since frogmouth
is running on Windows XP, which reputedly has a default socket receive
buffer size of only 8KB, it is not very surprising if this has put us over
the threshold where the receive buffer can overflow and drop messages.

The same mechanism could very easily explain the intermittent stats test
failures we've been seeing for years, since background processes such
as the bgwriter will sometimes send data concurrently with all this, and
could thus cause occasional buffer overflows.

Hence, insert some code into pgstat_init() to increase the stats socket's
receive buffer size to 100KB if it's less than that.  (On failure, emit a
LOG message, but keep going.)  Modern systems seem to have default sizes
in the range of 100KB-250KB, but older platforms don't.  I couldn't find
any platforms that wouldn't accept 100KB, so in theory this won't cause
any portability problems.

If this is successful at reducing the buildfarm failure rate in HEAD,
we should back-patch it, because it's certain that similar buffer overflows
happen in the field on platforms with small buffer sizes.  Going forward,
there might be an argument for trying to increase the buffer size even
more, but let's take a baby step first.

Discussion: https://postgr.es/m/22173.1494788088@sss.pgh.pa.us

src/backend/postmaster/pgstat.c

index d4feed127186b7bc748181168d5651fd3ef250a3..ba0ad3eb03aa470620122cea93105e350c711988 100644 (file)
@@ -93,6 +93,9 @@
 #define PGSTAT_POLL_LOOP_COUNT (PGSTAT_MAX_WAIT_TIME / PGSTAT_RETRY_DELAY)
 #define PGSTAT_INQ_LOOP_COUNT  (PGSTAT_INQ_INTERVAL / PGSTAT_RETRY_DELAY)
 
+/* Minimum receive buffer size for the collector's socket. */
+#define PGSTAT_MIN_RCVBUF              (100 * 1024)
+
 
 /* ----------
  * The initial size hints for the hash tables used in the collector.
@@ -574,6 +577,35 @@ retry2:
                goto startup_failed;
        }
 
+       /*
+        * Try to ensure that the socket's receive buffer is at least
+        * PGSTAT_MIN_RCVBUF bytes, so that it won't easily overflow and lose
+        * data.  Use of UDP protocol means that we are willing to lose data under
+        * heavy load, but we don't want it to happen just because of ridiculously
+        * small default buffer sizes (such as 8KB on older Windows versions).
+        */
+       {
+               int                     old_rcvbuf;
+               int                     new_rcvbuf;
+               ACCEPT_TYPE_ARG3 rcvbufsize = sizeof(old_rcvbuf);
+
+               if (getsockopt(pgStatSock, SOL_SOCKET, SO_RCVBUF,
+                                          (char *) &old_rcvbuf, &rcvbufsize) < 0)
+               {
+                       elog(LOG, "getsockopt(SO_RCVBUF) failed: %m");
+                       /* if we can't get existing size, always try to set it */
+                       old_rcvbuf = 0;
+               }
+
+               new_rcvbuf = PGSTAT_MIN_RCVBUF;
+               if (old_rcvbuf < new_rcvbuf)
+               {
+                       if (setsockopt(pgStatSock, SOL_SOCKET, SO_RCVBUF,
+                                                  (char *) &new_rcvbuf, sizeof(new_rcvbuf)) < 0)
+                               elog(LOG, "setsockopt(SO_RCVBUF) failed: %m");
+               }
+       }
+
        pg_freeaddrinfo_all(hints.ai_family, addrs);
 
        return;