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

Commit 3d70127

Browse files
committed
Perform an immediate shutdown if the postmaster.pid file is removed.
The postmaster now checks every minute or so (worst case, at most two minutes) that postmaster.pid is still there and still contains its own PID. If not, it performs an immediate shutdown, as though it had received SIGQUIT. The original goal behind this change was to ensure that failed buildfarm runs would get fully cleaned up, even if the test scripts had left a postmaster running, which is not an infrequent occurrence. When the buildfarm script removes a test postmaster's $PGDATA directory, its next check on postmaster.pid will fail and cause it to exit. Previously, manual intervention was often needed to get rid of such orphaned postmasters, since they'd block new test postmasters from obtaining the expected socket address. However, by checking postmaster.pid and not something else, we can provide additional robustness: manual removal of postmaster.pid is a frequent DBA mistake, and now we can at least limit the damage that will ensue if a new postmaster is started while the old one is still alive. Back-patch to all supported branches, since we won't get the desired improvement in buildfarm reliability otherwise.
1 parent d25c7d7 commit 3d70127

File tree

3 files changed

+115
-14
lines changed

3 files changed

+115
-14
lines changed

src/backend/postmaster/postmaster.c

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1597,9 +1597,10 @@ ServerLoop(void)
15971597
fd_set readmask;
15981598
int nSockets;
15991599
time_t now,
1600+
last_lockfile_recheck_time,
16001601
last_touch_time;
16011602

1602-
last_touch_time = time(NULL);
1603+
last_lockfile_recheck_time = last_touch_time = time(NULL);
16031604

16041605
nSockets = initMasks(&readmask);
16051606

@@ -1749,19 +1750,6 @@ ServerLoop(void)
17491750
if (StartWorkerNeeded || HaveCrashedWorker)
17501751
maybe_start_bgworker();
17511752

1752-
/*
1753-
* Touch Unix socket and lock files every 58 minutes, to ensure that
1754-
* they are not removed by overzealous /tmp-cleaning tasks. We assume
1755-
* no one runs cleaners with cutoff times of less than an hour ...
1756-
*/
1757-
now = time(NULL);
1758-
if (now - last_touch_time >= 58 * SECS_PER_MINUTE)
1759-
{
1760-
TouchSocketFiles();
1761-
TouchSocketLockFiles();
1762-
last_touch_time = now;
1763-
}
1764-
17651753
#ifdef HAVE_PTHREAD_IS_THREADED_NP
17661754

17671755
/*
@@ -1788,6 +1776,48 @@ ServerLoop(void)
17881776
/* reset flag so we don't SIGKILL again */
17891777
AbortStartTime = 0;
17901778
}
1779+
1780+
/*
1781+
* Lastly, check to see if it's time to do some things that we don't
1782+
* want to do every single time through the loop, because they're a
1783+
* bit expensive. Note that there's up to a minute of slop in when
1784+
* these tasks will be performed, since DetermineSleepTime() will let
1785+
* us sleep at most that long.
1786+
*/
1787+
now = time(NULL);
1788+
1789+
/*
1790+
* Once a minute, verify that postmaster.pid hasn't been removed or
1791+
* overwritten. If it has, we force a shutdown. This avoids having
1792+
* postmasters and child processes hanging around after their database
1793+
* is gone, and maybe causing problems if a new database cluster is
1794+
* created in the same place. It also provides some protection
1795+
* against a DBA foolishly removing postmaster.pid and manually
1796+
* starting a new postmaster. Data corruption is likely to ensue from
1797+
* that anyway, but we can minimize the damage by aborting ASAP.
1798+
*/
1799+
if (now - last_lockfile_recheck_time >= 1 * SECS_PER_MINUTE)
1800+
{
1801+
if (!RecheckDataDirLockFile())
1802+
{
1803+
ereport(LOG,
1804+
(errmsg("performing immediate shutdown because data directory lock file is invalid")));
1805+
kill(MyProcPid, SIGQUIT);
1806+
}
1807+
last_lockfile_recheck_time = now;
1808+
}
1809+
1810+
/*
1811+
* Touch Unix socket and lock files every 58 minutes, to ensure that
1812+
* they are not removed by overzealous /tmp-cleaning tasks. We assume
1813+
* no one runs cleaners with cutoff times of less than an hour ...
1814+
*/
1815+
if (now - last_touch_time >= 58 * SECS_PER_MINUTE)
1816+
{
1817+
TouchSocketFiles();
1818+
TouchSocketLockFiles();
1819+
last_touch_time = now;
1820+
}
17911821
}
17921822
}
17931823

src/backend/utils/init/miscinit.c

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,6 +1080,76 @@ AddToDataDirLockFile(int target_line, const char *str)
10801080
}
10811081

10821082

1083+
/*
1084+
* Recheck that the data directory lock file still exists with expected
1085+
* content. Return TRUE if the lock file appears OK, FALSE if it isn't.
1086+
*
1087+
* We call this periodically in the postmaster. The idea is that if the
1088+
* lock file has been removed or replaced by another postmaster, we should
1089+
* do a panic database shutdown. Therefore, we should return TRUE if there
1090+
* is any doubt: we do not want to cause a panic shutdown unnecessarily.
1091+
* Transient failures like EINTR or ENFILE should not cause us to fail.
1092+
* (If there really is something wrong, we'll detect it on a future recheck.)
1093+
*/
1094+
bool
1095+
RecheckDataDirLockFile(void)
1096+
{
1097+
int fd;
1098+
int len;
1099+
long file_pid;
1100+
char buffer[BLCKSZ];
1101+
1102+
fd = open(DIRECTORY_LOCK_FILE, O_RDWR | PG_BINARY, 0);
1103+
if (fd < 0)
1104+
{
1105+
/*
1106+
* There are many foreseeable false-positive error conditions. For
1107+
* safety, fail only on enumerated clearly-something-is-wrong
1108+
* conditions.
1109+
*/
1110+
switch (errno)
1111+
{
1112+
case ENOENT:
1113+
case ENOTDIR:
1114+
/* disaster */
1115+
ereport(LOG,
1116+
(errcode_for_file_access(),
1117+
errmsg("could not open file \"%s\": %m",
1118+
DIRECTORY_LOCK_FILE)));
1119+
return false;
1120+
default:
1121+
/* non-fatal, at least for now */
1122+
ereport(LOG,
1123+
(errcode_for_file_access(),
1124+
errmsg("could not open file \"%s\": %m; continuing anyway",
1125+
DIRECTORY_LOCK_FILE)));
1126+
return true;
1127+
}
1128+
}
1129+
len = read(fd, buffer, sizeof(buffer) - 1);
1130+
if (len < 0)
1131+
{
1132+
ereport(LOG,
1133+
(errcode_for_file_access(),
1134+
errmsg("could not read from file \"%s\": %m",
1135+
DIRECTORY_LOCK_FILE)));
1136+
close(fd);
1137+
return true; /* treat read failure as nonfatal */
1138+
}
1139+
buffer[len] = '\0';
1140+
close(fd);
1141+
file_pid = atol(buffer);
1142+
if (file_pid == getpid())
1143+
return true; /* all is well */
1144+
1145+
/* Trouble: someone's overwritten the lock file */
1146+
ereport(LOG,
1147+
(errmsg("lock file \"%s\" contains wrong PID: %ld instead of %ld",
1148+
DIRECTORY_LOCK_FILE, file_pid, (long) getpid())));
1149+
return false;
1150+
}
1151+
1152+
10831153
/*-------------------------------------------------------------------------
10841154
* Version checking support
10851155
*-------------------------------------------------------------------------

src/include/miscadmin.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,7 @@ extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster,
443443
const char *socketDir);
444444
extern void TouchSocketLockFiles(void);
445445
extern void AddToDataDirLockFile(int target_line, const char *str);
446+
extern bool RecheckDataDirLockFile(void);
446447
extern void ValidatePgVersion(const char *path);
447448
extern void process_shared_preload_libraries(void);
448449
extern void process_session_preload_libraries(void);

0 commit comments

Comments
 (0)