Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e73fe6e

Browse files
committed
Block signals while allocating DSM memory.
On Linux, we call posix_fallocate() on shm_open()'d memory to avoid later potential SIGBUS (see commit 899bd78). Based on field reports of systems stuck in an EINTR retry loop there, there, we made it possible to break out of that loop via slightly odd coding where the CHECK_FOR_INTERRUPTS() call was somewhat removed from the loop (see commit 422952e). On further reflection, that was not a great choice for at least two reasons: 1. If interrupts were held, the CHECK_FOR_INTERRUPTS() would do nothing and the EINTR error would be surfaced to the user. 2. If EINTR was reported but neither QueryCancelPending nor ProcDiePending was set, then we'd dutifully retry, but with a bit more understanding of how posix_fallocate() works, it's now clear that you can get into a loop that never terminates. posix_fallocate() is not a function that can do some of the job and tell you about progress if it's interrupted, it has to undo what it's done so far and report EINTR, and if signals keep arriving faster than it can complete (cf recovery conflict signals), you're stuck. Therefore, for now, we'll simply block most signals to guarantee progress. SIGQUIT is not blocked (see InitPostmasterChild()), because its expected handler doesn't return, and unblockable signals like SIGCONT are not expected to arrive at a high rate. For good measure, we'll include the ftruncate() call in the blocked region, and add a retry loop. Back-patch to all supported releases. Reported-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Reported-by: Nicola Contu <nicola.contu@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20220701154105.jjfutmngoedgiad3%40alvherre.pgsql
1 parent 7cdd0c2 commit e73fe6e

File tree

1 file changed

+22
-13
lines changed

1 file changed

+22
-13
lines changed

src/backend/storage/ipc/dsm_impl.c

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
#endif
6363

6464
#include "common/file_perm.h"
65+
#include "libpq/pqsignal.h" /* for PG_SETMASK macro */
6566
#include "miscadmin.h"
6667
#include "pgstat.h"
6768
#include "portability/mem.h"
@@ -303,14 +304,6 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size,
303304
shm_unlink(name);
304305
errno = save_errno;
305306

306-
/*
307-
* If we received a query cancel or termination signal, we will have
308-
* EINTR set here. If the caller said that errors are OK here, check
309-
* for interrupts immediately.
310-
*/
311-
if (errno == EINTR && elevel >= ERROR)
312-
CHECK_FOR_INTERRUPTS();
313-
314307
ereport(elevel,
315308
(errcode_for_dynamic_shared_memory(),
316309
errmsg("could not resize shared memory segment \"%s\" to %zu bytes: %m",
@@ -358,9 +351,21 @@ static int
358351
dsm_impl_posix_resize(int fd, off_t size)
359352
{
360353
int rc;
354+
int save_errno;
355+
356+
/*
357+
* Block all blockable signals, except SIGQUIT. posix_fallocate() can run
358+
* for quite a long time, and is an all-or-nothing operation. If we
359+
* allowed SIGUSR1 to interrupt us repeatedly (for example, due to recovery
360+
* conflicts), the retry loop might never succeed.
361+
*/
362+
PG_SETMASK(&BlockSig);
361363

362364
/* Truncate (or extend) the file to the requested size. */
363-
rc = ftruncate(fd, size);
365+
do
366+
{
367+
rc = ftruncate(fd, size);
368+
} while (rc < 0 && errno == EINTR);
364369

365370
/*
366371
* On Linux, a shm_open fd is backed by a tmpfs file. After resizing with
@@ -374,15 +379,15 @@ dsm_impl_posix_resize(int fd, off_t size)
374379
if (rc == 0)
375380
{
376381
/*
377-
* We may get interrupted. If so, just retry unless there is an
378-
* interrupt pending. This avoids the possibility of looping forever
379-
* if another backend is repeatedly trying to interrupt us.
382+
* We still use a traditional EINTR retry loop to handle SIGCONT.
383+
* posix_fallocate() doesn't restart automatically, and we don't want
384+
* this to fail if you attach a debugger.
380385
*/
381386
pgstat_report_wait_start(WAIT_EVENT_DSM_FILL_ZERO_WRITE);
382387
do
383388
{
384389
rc = posix_fallocate(fd, 0, size);
385-
} while (rc == EINTR && !(ProcDiePending || QueryCancelPending));
390+
} while (rc == EINTR);
386391
pgstat_report_wait_end();
387392

388393
/*
@@ -394,6 +399,10 @@ dsm_impl_posix_resize(int fd, off_t size)
394399
}
395400
#endif /* HAVE_POSIX_FALLOCATE && __linux__ */
396401

402+
save_errno = errno;
403+
PG_SETMASK(&UnBlockSig);
404+
errno = save_errno;
405+
397406
return rc;
398407
}
399408

0 commit comments

Comments
 (0)