Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.92) (envelope-from ) id 1j60bj-0002zQ-DP for pgsql-hackers@arkaria.postgresql.org; Sun, 23 Feb 2020 23:24:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1j60bg-0005cT-6u for pgsql-hackers@arkaria.postgresql.org; Sun, 23 Feb 2020 23:24:16 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1j60bf-0005cM-SN for pgsql-hackers@lists.postgresql.org; Sun, 23 Feb 2020 23:24:15 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1j60bc-0002NV-0M for pgsql-hackers@lists.postgresql.org; Sun, 23 Feb 2020 23:24:14 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id 01NNO7rl027650; Sun, 23 Feb 2020 18:24:07 -0500 From: Tom Lane To: Thomas Munro cc: =?UTF-8?Q?R=C3=A9mi_Zara?= , Michael Paquier , PostgreSQL Hackers Subject: Re: pgsql: Add kqueue(2) support to the WaitEventSet API. In-reply-to: <6230.1582494552@sss.pgh.pa.us> References: <20200220072435.GM2288@paquier.xyz> <08364CCB-07F5-423E-8CE0-ABF0195C4CA2@mac.com> <17106.1582224047@sss.pgh.pa.us> <19710.1582227844@sss.pgh.pa.us> <20263.1582228617@sss.pgh.pa.us> <26174.1582236314@sss.pgh.pa.us> <6610.1582242943@sss.pgh.pa.us> <18091.1582259407@sss.pgh.pa.us> <6230.1582494552@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Sun, 23 Feb 2020 16:49:12 -0500" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <27630.1582500222.0@sss.pgh.pa.us> Date: Sun, 23 Feb 2020 18:24:07 -0500 Message-ID: <27649.1582500247@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <27630.1582500222.1@sss.pgh.pa.us> I wrote: > Here's a draft patch that does it like that. On reflection, trying to make ReserveExternalFD serve two different use-cases was pretty messy. Here's a version that splits it into two functions. I also took the trouble to fix dblink. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="account-honestly-for-external-FD-usage-2.patch"; charset="us-ascii" Content-ID: <27630.1582500222.2@sss.pgh.pa.us> Content-Description: account-honestly-for-external-FD-usage-2.patch Content-Transfer-Encoding: quoted-printable diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index c1155e3..a5f69fc 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -200,12 +200,27 @@ dblink_get_conn(char *conname_or_str, if (connstr =3D=3D NULL) connstr =3D conname_or_str; dblink_connstr_check(connstr); + + /* + * We must obey fd.c's limit on non-virtual file descriptors. Assume + * that a PGconn represents one long-lived FD. (Doing this here also + * ensures that VFDs are closed if needed to make room.) + */ + if (!AcquireExternalFD()) + ereport(ERROR, + (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), + errmsg("could not establish connection"), + errdetail("There are too many open files."))); + + /* OK to make connection */ conn =3D PQconnectdb(connstr); + if (PQstatus(conn) =3D=3D CONNECTION_BAD) { char *msg =3D pchomp(PQerrorMessage(conn)); = PQfinish(conn); + ReleaseExternalFD(); ereport(ERROR, (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), errmsg("could not establish connection"), @@ -282,12 +297,26 @@ dblink_connect(PG_FUNCTION_ARGS) = /* check password in connection string if not superuser */ dblink_connstr_check(connstr); + + /* + * We must obey fd.c's limit on non-virtual file descriptors. Assume th= at + * a PGconn represents one long-lived FD. (Doing this here also ensures + * that VFDs are closed if needed to make room.) + */ + if (!AcquireExternalFD()) + ereport(ERROR, + (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), + errmsg("could not establish connection"), + errdetail("There are too many open files."))); + + /* OK to make connection */ conn =3D PQconnectdb(connstr); = if (PQstatus(conn) =3D=3D CONNECTION_BAD) { msg =3D pchomp(PQerrorMessage(conn)); PQfinish(conn); + ReleaseExternalFD(); if (rconn) pfree(rconn); = @@ -312,7 +341,10 @@ dblink_connect(PG_FUNCTION_ARGS) else { if (pconn->conn) + { PQfinish(pconn->conn); + ReleaseExternalFD(); + } pconn->conn =3D conn; } = @@ -346,6 +378,7 @@ dblink_disconnect(PG_FUNCTION_ARGS) dblink_conn_not_avail(conname); = PQfinish(conn); + ReleaseExternalFD(); if (rconn) { deleteConnection(conname); @@ -780,7 +813,10 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool = is_async) { /* if needed, close the connection to the database */ if (freeconn) + { PQfinish(conn); + ReleaseExternalFD(); + } } PG_END_TRY(); = @@ -1458,7 +1494,10 @@ dblink_exec(PG_FUNCTION_ARGS) { /* if needed, close the connection to the database */ if (freeconn) + { PQfinish(conn); + ReleaseExternalFD(); + } } PG_END_TRY(); = @@ -2563,6 +2602,7 @@ createNewConnection(const char *name, remoteConn *rc= onn) if (found) { PQfinish(rconn->conn); + ReleaseExternalFD(); pfree(rconn); = ereport(ERROR, @@ -2604,6 +2644,7 @@ dblink_security_check(PGconn *conn, remoteConn *rcon= n) if (!PQconnectionUsedPassword(conn)) { PQfinish(conn); + ReleaseExternalFD(); if (rconn) pfree(rconn); = diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/conn= ection.c index 29c811a..74576d7 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -20,6 +20,7 @@ #include "miscadmin.h" #include "pgstat.h" #include "postgres_fdw.h" +#include "storage/fd.h" #include "storage/latch.h" #include "utils/hsearch.h" #include "utils/inval.h" @@ -259,10 +260,27 @@ connect_pg_server(ForeignServer *server, UserMapping= *user) = keywords[n] =3D values[n] =3D NULL; = - /* verify connection parameters and make connection */ + /* verify the set of connection parameters */ check_conn_params(keywords, values, user); = + /* + * We must obey fd.c's limit on non-virtual file descriptors. Assume + * that a PGconn represents one long-lived FD. (Doing this here also + * ensures that VFDs are closed if needed to make room.) + */ + if (!AcquireExternalFD()) + ereport(ERROR, + (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), + errmsg("could not connect to server \"%s\"", + server->servername), + errdetail("There are too many open files."))); + + /* OK to make connection */ conn =3D PQconnectdbParams(keywords, values, false); + + if (!conn) + ReleaseExternalFD(); /* because the PG_CATCH block won't */ + if (!conn || PQstatus(conn) !=3D CONNECTION_OK) ereport(ERROR, (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), @@ -294,7 +312,10 @@ connect_pg_server(ForeignServer *server, UserMapping = *user) { /* Release PGconn data structure if we managed to create one */ if (conn) + { PQfinish(conn); + ReleaseExternalFD(); + } PG_RE_THROW(); } PG_END_TRY(); @@ -312,6 +333,7 @@ disconnect_pg_server(ConnCacheEntry *entry) { PQfinish(entry->conn); entry->conn =3D NULL; + ReleaseExternalFD(); } } = diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transa= m/xlog.c index fd527f2..d19408b 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -774,6 +774,7 @@ static const char *const xlogSourceNames[] =3D {"any",= "archive", "pg_wal", "strea * openLogFile is -1 or a kernel FD for an open log file segment. * openLogSegNo identifies the segment. These variables are only used to * write the XLOG, and so will normally refer to the active segment. + * Note: call Reserve/ReleaseExternalFD to track consumption of this FD. */ static int openLogFile =3D -1; static XLogSegNo openLogSegNo =3D 0; @@ -785,6 +786,9 @@ static XLogSegNo openLogSegNo =3D 0; * will be just past that page. readLen indicates how much of the current * page has been read into readBuf, and readSource indicates where we got * the currently open file from. + * Note: we could use Reserve/ReleaseExternalFD to track consumption of + * this FD too; but it doesn't currently seem worthwhile, since the XLOG = is + * not read by general-purpose sessions. */ static int readFile =3D -1; static XLogSegNo readSegNo =3D 0; @@ -2447,6 +2451,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) /* create/use new log file */ use_existent =3D true; openLogFile =3D XLogFileInit(openLogSegNo, &use_existent, true); + ReserveExternalFD(); } = /* Make sure we have the current logfile open */ @@ -2455,6 +2460,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) XLByteToPrevSeg(LogwrtResult.Write, openLogSegNo, wal_segment_size); openLogFile =3D XLogFileOpen(openLogSegNo); + ReserveExternalFD(); } = /* Add current page to the set of pending pages-to-dump */ @@ -2605,6 +2611,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) XLByteToPrevSeg(LogwrtResult.Write, openLogSegNo, wal_segment_size); openLogFile =3D XLogFileOpen(openLogSegNo); + ReserveExternalFD(); } = issue_xlog_fsync(openLogFile, openLogSegNo); @@ -3811,6 +3818,7 @@ XLogFileClose(void) } = openLogFile =3D -1; + ReleaseExternalFD(); } = /* @@ -5224,6 +5232,11 @@ BootStrapXLOG(void) use_existent =3D false; openLogFile =3D XLogFileInit(1, &use_existent, false); = + /* + * We needn't bother with Reserve/ReleaseExternalFD here, since we'll + * close the file again in a moment. + */ + /* Write the first page with the initial record */ errno =3D 0; pgstat_report_wait_start(WAIT_EVENT_WAL_BOOTSTRAP_WRITE); diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgst= at.c index 59dc4f3..462b4d7 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -602,6 +602,9 @@ retry2: = pg_freeaddrinfo_all(hints.ai_family, addrs); = + /* Now that we have a long-lived socket, tell fd.c about it. */ + ReserveExternalFD(); + return; = startup_failed: diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/= postmaster.c index b3986be..cd61665 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -2554,9 +2554,14 @@ ClosePostmasterPorts(bool am_syslogger) (errcode_for_file_access(), errmsg_internal("could not close postmaster death monitoring pipe in= child process: %m"))); postmaster_alive_fds[POSTMASTER_FD_OWN] =3D -1; + /* Notify fd.c that we released one pipe FD. */ + ReleaseExternalFD(); #endif = - /* Close the listen sockets */ + /* + * Close the postmaster's listen sockets. These aren't tracked by fd.c, + * so we don't call ReleaseExternalFD() here. + */ for (i =3D 0; i < MAXLISTEN; i++) { if (ListenSocket[i] !=3D PGINVALID_SOCKET) @@ -2566,7 +2571,10 @@ ClosePostmasterPorts(bool am_syslogger) } } = - /* If using syslogger, close the read side of the pipe */ + /* + * If using syslogger, close the read side of the pipe. We don't bother + * tracking this in fd.c, either. + */ if (!am_syslogger) { #ifndef WIN32 @@ -4279,6 +4287,9 @@ BackendInitialize(Port *port) /* Save port etc. for ps status */ MyProcPort =3D port; = + /* Tell fd.c about the long-lived FD associated with the port */ + ReserveExternalFD(); + /* * PreAuthDelay is a debugging aid for investigating problems in the * authentication cycle: it can be set in postgresql.conf to allow time = to @@ -6442,6 +6453,20 @@ restore_backend_variables(BackendParameters *param,= Port *port) strlcpy(pkglib_path, param->pkglib_path, MAXPGPATH); = strlcpy(ExtraOptions, param->ExtraOptions, MAXPGPATH); + + /* + * We need to restore fd.c's counts of externally-opened FDs; to avoid + * confusion, be sure to do this after restoring max_safe_fds. (Note: + * BackendInitialize will handle this for port->sock.) + */ +#ifndef WIN32 + if (postmaster_alive_fds[0] >=3D 0) + ReserveExternalFD(); + if (postmaster_alive_fds[1] >=3D 0) + ReserveExternalFD(); +#endif + if (pgStatSock !=3D PGINVALID_SOCKET) + ReserveExternalFD(); } = = @@ -6584,6 +6609,10 @@ InitPostmasterDeathWatchHandle(void) (errcode_for_file_access(), errmsg_internal("could not create pipe to monitor postmaster death: = %m"))); = + /* Notify fd.c that we've eaten two FDs for the pipe. */ + ReserveExternalFD(); + ReserveExternalFD(); + /* * Set O_NONBLOCK to allow testing for the fd's presence with a read() * call. diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/s= yslogger.c index b2b69a7..cf7b535 100644 --- a/src/backend/postmaster/syslogger.c +++ b/src/backend/postmaster/syslogger.c @@ -562,6 +562,11 @@ SysLogger_Start(void) * This means the postmaster must continue to hold the read end of the * pipe open, so we can pass it down to the reincarnated syslogger. This * is a bit klugy but we have little choice. + * + * Also note that we don't bother counting the pipe FDs by calling + * Reserve/ReleaseExternalFD. There's no real need to account for them + * accurately in the postmaster or syslogger process, and both ends of t= he + * pipe will wind up closed in all other postmaster children. */ #ifndef WIN32 if (syslogPipe[0] < 0) diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index b5f4df6..34f7443 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -61,6 +61,12 @@ * BasicOpenFile, it is solely the caller's responsibility to close the f= ile * descriptor by calling close(2). * + * If a non-virtual file descriptor needs to be held open for any length = of + * time, report it to fd.c by calling AcquireExternalFD or ReserveExterna= lFD + * (and eventually ReleaseExternalFD), so that we can take it into accoun= t + * while deciding how many VFDs can be open. This applies to FDs obtaine= d + * with BasicOpenFile as well as those obtained without use of any fd.c A= PI. + * *-----------------------------------------------------------------------= -- */ = @@ -103,8 +109,8 @@ /* * We must leave some file descriptors free for system(), the dynamic loa= der, * and other code that tries to open files without consulting fd.c. This - * is the number left free. (While we can be pretty sure we won't get - * EMFILE, there's never any guarantee that we won't get ENFILE due to + * is the number left free. (While we try fairly hard to prevent EMFILE + * errors, there's never any guarantee that we won't get ENFILE due to * other processes chewing up FDs. So it's a bad idea to try to open fil= es * without consulting fd.c. Nonetheless we cannot control all code.) * @@ -119,9 +125,12 @@ = /* * If we have fewer than this many usable FDs after allowing for the rese= rved - * ones, choke. + * ones, choke. (This value is chosen to work with "ulimit -n 64", but n= ot + * much less than that. Note that this value ensures numExternalFDs can = be + * at least 16; as of this writing, the contrib/postgres_fdw regression t= ests + * will not pass unless that can grow to at least 14.) */ -#define FD_MINFREE 10 +#define FD_MINFREE 48 = /* * A number of platforms allow individual processes to open many more fil= es @@ -132,8 +141,8 @@ int max_files_per_process =3D 1000; = /* - * Maximum number of file descriptors to open for either VFD entries or - * AllocateFile/AllocateDir/OpenTransientFile operations. This is initia= lized + * Maximum number of file descriptors to open for operations that fd.c kn= ows + * about (VFDs, AllocateFile etc, or "external" FDs). This is initialize= d * to a conservative value, and remains that way indefinitely in bootstra= p or * standalone-backend cases. In normal postmaster operation, the postmas= ter * calls set_max_safe_fds() late in initialization to update the value, a= nd @@ -142,7 +151,7 @@ int max_files_per_process =3D 1000; * Note: the value of max_files_per_process is taken into account while * setting this variable, and so need not be tested separately. */ -int max_safe_fds =3D 32; /* default if not changed */ +int max_safe_fds =3D FD_MINFREE; /* default if not changed */ = /* Whether it is safe to continue running after fsync() fails. */ bool data_sync_retry =3D false; @@ -244,6 +253,11 @@ static int maxAllocatedDescs =3D 0; static AllocateDesc *allocatedDescs =3D NULL; = /* + * Number of open "external" FDs reported to Reserve/ReleaseExternalFD. + */ +static int numExternalFDs =3D 0; + +/* * Number of temporary files opened during the current session; * this is used in generation of tempfile names. */ @@ -1025,6 +1039,80 @@ tryAgain: return -1; /* failure */ } = +/* + * AcquireExternalFD - attempt to reserve an external file descriptor + * + * This should be used by callers that need to hold a file descriptor ope= n + * over more than a short interval, but cannot use any of the other facil= ities + * provided by this module. + * + * The difference between this and the underlying ReserveExternalFD funct= ion + * is that this will report failure (by setting errno and returning false= ) + * if "too many" external FDs are already reserved. This should be used = in + * any code where the total number of FDs to be reserved is not predictab= le + * and small. + */ +bool +AcquireExternalFD(void) +{ + /* + * We don't want more than max_safe_fds / 3 FDs to be consumed for + * "external" FDs. + */ + if (numExternalFDs < max_safe_fds / 3) + { + ReserveExternalFD(); + return true; + } + errno =3D EMFILE; + return false; +} + +/* + * ReserveExternalFD - report external consumption of a file descriptor + * + * This should be used by callers that need to hold a file descriptor ope= n + * over more than a short interval, but cannot use any of the other facil= ities + * provided by this module. This just tracks the use of the FD and close= s + * VFDs if needed to ensure we keep NUM_RESERVED_FDS FDs available. + * + * Call this directly only in code where failure to reserve the FD would = be + * fatal; for example, the WAL-writing code does so, since the alternativ= e is + * session failure. Also, it's very unwise to do so in code that could + * consume more than one FD per process. + * + * Note: as long as everybody plays nice so that NUM_RESERVED_FDS FDs rem= ain + * available, it doesn't matter too much whether this is called before or + * after actually opening the FD; but doing so beforehand reduces the ris= k of + * an EMFILE failure if not everybody played nice. In any case, it's sol= ely + * caller's responsibility to keep the external-FD count in sync with rea= lity. + */ +void +ReserveExternalFD(void) +{ + /* + * Release VFDs if needed to stay safe. Because we do this before + * incrementing numExternalFDs, the final state will be as desired, i.e.= , + * nfile + numAllocatedDescs + numExternalFDs <=3D max_safe_fds. + */ + ReleaseLruFiles(); + + numExternalFDs++; +} + +/* + * ReleaseExternalFD - report release of an external file descriptor + * + * This is guaranteed not to change errno, so it can be used in failure p= aths. + */ +void +ReleaseExternalFD(void) +{ + Assert(numExternalFDs > 0); + numExternalFDs--; +} + + #if defined(FDDEBUG) = static void @@ -1185,7 +1273,7 @@ ReleaseLruFile(void) static void ReleaseLruFiles(void) { - while (nfile + numAllocatedDescs >=3D max_safe_fds) + while (nfile + numAllocatedDescs + numExternalFDs >=3D max_safe_fds) { if (!ReleaseLruFile()) break; @@ -2176,13 +2264,13 @@ reserveAllocatedDesc(void) = /* * If the array hasn't yet been created in the current process, initiali= ze - * it with FD_MINFREE / 2 elements. In many scenarios this is as many a= s + * it with FD_MINFREE / 3 elements. In many scenarios this is as many a= s * we will ever need, anyway. We don't want to look at max_safe_fds * immediately because set_max_safe_fds() may not have run yet. */ if (allocatedDescs =3D=3D NULL) { - newMax =3D FD_MINFREE / 2; + newMax =3D FD_MINFREE / 3; newDescs =3D (AllocateDesc *) malloc(newMax * sizeof(AllocateDesc)); /* Out of memory already? Treat as fatal error. */ if (newDescs =3D=3D NULL) @@ -2200,10 +2288,12 @@ reserveAllocatedDesc(void) * * We mustn't let allocated descriptors hog all the available FDs, and i= n * practice we'd better leave a reasonable number of FDs for VFD use. S= o - * set the maximum to max_safe_fds / 2. (This should certainly be at - * least as large as the initial size, FD_MINFREE / 2.) + * set the maximum to max_safe_fds / 3. (This should certainly be at + * least as large as the initial size, FD_MINFREE / 3, so we aren't + * tightening the restriction here.) Recall that "external" FDs are + * allowed to consume another third of max_safe_fds. */ - newMax =3D max_safe_fds / 2; + newMax =3D max_safe_fds / 3; if (newMax > maxAllocatedDescs) { newDescs =3D (AllocateDesc *) realloc(allocatedDescs, diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/= dsm_impl.c index 138bdec..1972aec 100644 --- a/src/backend/storage/ipc/dsm_impl.c +++ b/src/backend/storage/ipc/dsm_impl.c @@ -247,14 +247,17 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size re= quest_size, /* * Create new segment or open an existing one for attach. * - * Even though we're not going through fd.c, we should be safe against - * running out of file descriptors, because of NUM_RESERVED_FDS. We're - * only opening one extra descriptor here, and we'll close it before - * returning. + * Even though we will close the FD before returning, it seems desirable + * to use Reserve/ReleaseExternalFD, to reduce the probability of EMFILE + * failure. The fact that we won't hold the FD open long justifies usin= g + * ReserveExternalFD rather than AcquireExternalFD, though. */ + ReserveExternalFD(); + flags =3D O_RDWR | (op =3D=3D DSM_OP_CREATE ? O_CREAT | O_EXCL : 0); if ((fd =3D shm_open(name, flags, PG_FILE_MODE_OWNER)) =3D=3D -1) { + ReleaseExternalFD(); if (errno !=3D EEXIST) ereport(elevel, (errcode_for_dynamic_shared_memory(), @@ -278,6 +281,7 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size requ= est_size, /* Back out what's already been done. */ save_errno =3D errno; close(fd); + ReleaseExternalFD(); errno =3D save_errno; = ereport(elevel, @@ -295,6 +299,7 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size requ= est_size, /* Back out what's already been done. */ save_errno =3D errno; close(fd); + ReleaseExternalFD(); shm_unlink(name); errno =3D save_errno; = @@ -323,6 +328,7 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size requ= est_size, /* Back out what's already been done. */ save_errno =3D errno; close(fd); + ReleaseExternalFD(); if (op =3D=3D DSM_OP_CREATE) shm_unlink(name); errno =3D save_errno; @@ -336,6 +342,7 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size requ= est_size, *mapped_address =3D address; *mapped_size =3D request_size; close(fd); + ReleaseExternalFD(); = return true; } diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/lat= ch.c index cbd4952..046ca5c 100644 --- a/src/backend/storage/ipc/latch.c +++ b/src/backend/storage/ipc/latch.c @@ -51,6 +51,7 @@ #include "port/atomics.h" #include "portability/instr_time.h" #include "postmaster/postmaster.h" +#include "storage/fd.h" #include "storage/ipc.h" #include "storage/latch.h" #include "storage/pmsignal.h" @@ -187,6 +188,9 @@ InitializeLatchSupport(void) /* Clean up, just for safety's sake; we'll set these below */ selfpipe_readfd =3D selfpipe_writefd =3D -1; selfpipe_owner_pid =3D 0; + /* Keep fd.c's accounting straight */ + ReleaseExternalFD(); + ReleaseExternalFD(); } else { @@ -194,6 +198,7 @@ InitializeLatchSupport(void) * Postmaster didn't create a self-pipe ... or else we're in an * EXEC_BACKEND build, in which case it doesn't matter since the * postmaster's pipe FDs were closed by the action of FD_CLOEXEC. + * fd.c won't have state to clean up, either. */ Assert(selfpipe_readfd =3D=3D -1); } @@ -228,6 +233,10 @@ InitializeLatchSupport(void) selfpipe_readfd =3D pipefd[0]; selfpipe_writefd =3D pipefd[1]; selfpipe_owner_pid =3D MyProcPid; + + /* Tell fd.c about these two long-lived FDs */ + ReserveExternalFD(); + ReserveExternalFD(); #else /* currently, nothing to do here for Windows */ #endif @@ -604,24 +613,57 @@ CreateWaitEventSet(MemoryContext context, int nevent= s) set->exit_on_postmaster_death =3D false; = #if defined(WAIT_USE_EPOLL) + if (!AcquireExternalFD()) + { + /* treat this as though epoll_create1 itself returned EMFILE */ + elog(ERROR, "epoll_create1 failed: %m"); + } #ifdef EPOLL_CLOEXEC set->epoll_fd =3D epoll_create1(EPOLL_CLOEXEC); if (set->epoll_fd < 0) + { + ReleaseExternalFD(); elog(ERROR, "epoll_create1 failed: %m"); + } #else /* cope with ancient glibc lacking epoll_create1 (e.g., RHEL5) */ set->epoll_fd =3D epoll_create(nevents); if (set->epoll_fd < 0) + { + ReleaseExternalFD(); elog(ERROR, "epoll_create failed: %m"); + } if (fcntl(set->epoll_fd, F_SETFD, FD_CLOEXEC) =3D=3D -1) + { + int save_errno =3D errno; + + close(set->epoll_fd); + ReleaseExternalFD(); + errno =3D save_errno; elog(ERROR, "fcntl(F_SETFD) failed on epoll descriptor: %m"); + } #endif /* EPOLL_CLOEXEC */ #elif defined(WAIT_USE_KQUEUE) + if (!AcquireExternalFD()) + { + /* treat this as though kqueue itself returned EMFILE */ + elog(ERROR, "kqueue failed: %m"); + } set->kqueue_fd =3D kqueue(); if (set->kqueue_fd < 0) + { + ReleaseExternalFD(); elog(ERROR, "kqueue failed: %m"); + } if (fcntl(set->kqueue_fd, F_SETFD, FD_CLOEXEC) =3D=3D -1) + { + int save_errno =3D errno; + + close(set->kqueue_fd); + ReleaseExternalFD(); + errno =3D save_errno; elog(ERROR, "fcntl(F_SETFD) failed on kqueue descriptor: %m"); + } set->report_postmaster_not_running =3D false; #elif defined(WAIT_USE_WIN32) = @@ -655,8 +697,10 @@ FreeWaitEventSet(WaitEventSet *set) { #if defined(WAIT_USE_EPOLL) close(set->epoll_fd); + ReleaseExternalFD(); #elif defined(WAIT_USE_KQUEUE) close(set->kqueue_fd); + ReleaseExternalFD(); #elif defined(WAIT_USE_WIN32) WaitEvent *cur_event; = diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index 51e2ece..2085c62 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -35,6 +35,10 @@ * Likewise, use AllocateDir/FreeDir, not opendir/closedir, to allocate * open directories (DIR*), and OpenTransientFile/CloseTransientFile for = an * unbuffered file descriptor. + * + * If you really can't use any of the above, at least call AcquireExterna= lFD + * or ReserveExternalFD to report any file descriptors that are held for = any + * length of time. Failure to do so risks unnecessary EMFILE errors. */ #ifndef FD_H #define FD_H @@ -120,7 +124,12 @@ extern int CloseTransientFile(int fd); extern int BasicOpenFile(const char *fileName, int fileFlags); extern int BasicOpenFilePerm(const char *fileName, int fileFlags, mode_t = fileMode); = - /* Make a directory with default permissions */ +/* Use these for other cases, and also for long-lived BasicOpenFile FDs *= / +extern bool AcquireExternalFD(void); +extern void ReserveExternalFD(void); +extern void ReleaseExternalFD(void); + +/* Make a directory with default permissions */ extern int MakePGDirectory(const char *directoryName); = /* Miscellaneous support routines */ ------- =_aaaaaaaaaa0--