Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit3d47551

Browse files
committed
Account explicitly for long-lived FDs that are allocated outside fd.c.
The comments in fd.c have long claimed that all file allocations shouldgo through that module, but in reality that's not always practical.fd.c doesn't supply APIs for invoking some FD-producing syscalls likepipe() or epoll_create(); and the APIs it does supply for non-virtualFDs are mostly insistent on releasing those FDs at transaction end;and in some cases the actual open() call is in code that can't be madeto use fd.c, such as libpq.This has led to a situation where, in a modern server, there are likelyto be seven or so long-lived FDs per backend process that are not knownto fd.c. Since NUM_RESERVED_FDS is only 10, that meant we had *very*few spare FDs if max_files_per_process is >= the system ulimit andfd.c had opened all the files it thought it safely could. Thecontrib/postgres_fdw regression test, in particular, could easily bemade to fall over by running it under a restrictive ulimit.To improve matters, invent functions Acquire/Reserve/ReleaseExternalFDthat allow outside callers to tell fd.c that they have or want to allocatea FD that's not directly managed by fd.c. Add calls to track all thefixed FDs in a standard backend session, so that we are honestlyguaranteeing that NUM_RESERVED_FDS FDs remain unused below the EMFILElimit in a backend's idle state. The coding rules for these functions saythat there's no need to call them in code that just allocates one FD overa fairly short interval; we can dip into NUM_RESERVED_FDS for such cases.That means that there aren't all that many places where we need to worry.But postgres_fdw and dblink must use this facility to account forlong-lived FDs consumed by libpq connections. There may be other placeswhere it's worth doing such accounting, too, but this seems like enoughto solve the immediate problem.Internally to fd.c, "external" FDs are limited to max_safe_fds/3 FDs.(Callers can choose to ignore this limit, but of course it's unwiseto do so except for fixed file allocations.) I also reduced the limiton "allocated" files to max_safe_fds/3 FDs (it had been max_safe_fds/2).Conceivably a smarter rule could be used here --- but in practice,on reasonable systems, max_safe_fds should be large enough that thisisn't much of an issue, so KISS for now. To avoid possible regressionin the number of external or allocated files that can be opened,increase FD_MINFREE and the lower limit on max_files_per_process alittle bit; we now insist that the effective "ulimit -n" be at least 64.This seems like pretty clearly a bug fix, but in view of the lack offield complaints, I'll refrain from risking a back-patch.Discussion:https://postgr.es/m/E1izCmM-0005pV-Co@gemulon.postgresql.org
1 parent1420617 commit3d47551

File tree

12 files changed

+304
-23
lines changed

12 files changed

+304
-23
lines changed

‎contrib/dblink/dblink.c

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,12 +200,33 @@ dblink_get_conn(char *conname_or_str,
200200
if (connstr==NULL)
201201
connstr=conname_or_str;
202202
dblink_connstr_check(connstr);
203+
204+
/*
205+
* We must obey fd.c's limit on non-virtual file descriptors. Assume
206+
* that a PGconn represents one long-lived FD. (Doing this here also
207+
* ensures that VFDs are closed if needed to make room.)
208+
*/
209+
if (!AcquireExternalFD())
210+
ereport(ERROR,
211+
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
212+
errmsg("could not establish connection"),
213+
errdetail("There are too many open files on the local server."),
214+
#ifndefWIN32
215+
errhint("Raise the server's max_files_per_process and/or \"ulimit -n\" limits.")
216+
#else
217+
errhint("Raise the server's max_files_per_process setting.")
218+
#endif
219+
));
220+
221+
/* OK to make connection */
203222
conn=PQconnectdb(connstr);
223+
204224
if (PQstatus(conn)==CONNECTION_BAD)
205225
{
206226
char*msg=pchomp(PQerrorMessage(conn));
207227

208228
PQfinish(conn);
229+
ReleaseExternalFD();
209230
ereport(ERROR,
210231
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
211232
errmsg("could not establish connection"),
@@ -282,12 +303,32 @@ dblink_connect(PG_FUNCTION_ARGS)
282303

283304
/* check password in connection string if not superuser */
284305
dblink_connstr_check(connstr);
306+
307+
/*
308+
* We must obey fd.c's limit on non-virtual file descriptors. Assume that
309+
* a PGconn represents one long-lived FD. (Doing this here also ensures
310+
* that VFDs are closed if needed to make room.)
311+
*/
312+
if (!AcquireExternalFD())
313+
ereport(ERROR,
314+
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
315+
errmsg("could not establish connection"),
316+
errdetail("There are too many open files on the local server."),
317+
#ifndefWIN32
318+
errhint("Raise the server's max_files_per_process and/or \"ulimit -n\" limits.")
319+
#else
320+
errhint("Raise the server's max_files_per_process setting.")
321+
#endif
322+
));
323+
324+
/* OK to make connection */
285325
conn=PQconnectdb(connstr);
286326

287327
if (PQstatus(conn)==CONNECTION_BAD)
288328
{
289329
msg=pchomp(PQerrorMessage(conn));
290330
PQfinish(conn);
331+
ReleaseExternalFD();
291332
if (rconn)
292333
pfree(rconn);
293334

@@ -312,7 +353,10 @@ dblink_connect(PG_FUNCTION_ARGS)
312353
else
313354
{
314355
if (pconn->conn)
356+
{
315357
PQfinish(pconn->conn);
358+
ReleaseExternalFD();
359+
}
316360
pconn->conn=conn;
317361
}
318362

@@ -346,6 +390,7 @@ dblink_disconnect(PG_FUNCTION_ARGS)
346390
dblink_conn_not_avail(conname);
347391

348392
PQfinish(conn);
393+
ReleaseExternalFD();
349394
if (rconn)
350395
{
351396
deleteConnection(conname);
@@ -780,7 +825,10 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
780825
{
781826
/* if needed, close the connection to the database */
782827
if (freeconn)
828+
{
783829
PQfinish(conn);
830+
ReleaseExternalFD();
831+
}
784832
}
785833
PG_END_TRY();
786834

@@ -1458,7 +1506,10 @@ dblink_exec(PG_FUNCTION_ARGS)
14581506
{
14591507
/* if needed, close the connection to the database */
14601508
if (freeconn)
1509+
{
14611510
PQfinish(conn);
1511+
ReleaseExternalFD();
1512+
}
14621513
}
14631514
PG_END_TRY();
14641515

@@ -2563,6 +2614,7 @@ createNewConnection(const char *name, remoteConn *rconn)
25632614
if (found)
25642615
{
25652616
PQfinish(rconn->conn);
2617+
ReleaseExternalFD();
25662618
pfree(rconn);
25672619

25682620
ereport(ERROR,
@@ -2604,6 +2656,7 @@ dblink_security_check(PGconn *conn, remoteConn *rconn)
26042656
if (!PQconnectionUsedPassword(conn))
26052657
{
26062658
PQfinish(conn);
2659+
ReleaseExternalFD();
26072660
if (rconn)
26082661
pfree(rconn);
26092662

‎contrib/postgres_fdw/connection.c

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include"miscadmin.h"
2121
#include"pgstat.h"
2222
#include"postgres_fdw.h"
23+
#include"storage/fd.h"
2324
#include"storage/latch.h"
2425
#include"utils/hsearch.h"
2526
#include"utils/inval.h"
@@ -259,10 +260,33 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
259260

260261
keywords[n]=values[n]=NULL;
261262

262-
/* verifyconnection parameters and makeconnection */
263+
/* verifythe set ofconnection parameters */
263264
check_conn_params(keywords,values,user);
264265

266+
/*
267+
* We must obey fd.c's limit on non-virtual file descriptors. Assume
268+
* that a PGconn represents one long-lived FD. (Doing this here also
269+
* ensures that VFDs are closed if needed to make room.)
270+
*/
271+
if (!AcquireExternalFD())
272+
ereport(ERROR,
273+
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
274+
errmsg("could not connect to server \"%s\"",
275+
server->servername),
276+
errdetail("There are too many open files on the local server."),
277+
#ifndefWIN32
278+
errhint("Raise the server's max_files_per_process and/or \"ulimit -n\" limits.")
279+
#else
280+
errhint("Raise the server's max_files_per_process setting.")
281+
#endif
282+
));
283+
284+
/* OK to make connection */
265285
conn=PQconnectdbParams(keywords,values, false);
286+
287+
if (!conn)
288+
ReleaseExternalFD();/* because the PG_CATCH block won't */
289+
266290
if (!conn||PQstatus(conn)!=CONNECTION_OK)
267291
ereport(ERROR,
268292
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
@@ -294,7 +318,10 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
294318
{
295319
/* Release PGconn data structure if we managed to create one */
296320
if (conn)
321+
{
297322
PQfinish(conn);
323+
ReleaseExternalFD();
324+
}
298325
PG_RE_THROW();
299326
}
300327
PG_END_TRY();
@@ -312,6 +339,7 @@ disconnect_pg_server(ConnCacheEntry *entry)
312339
{
313340
PQfinish(entry->conn);
314341
entry->conn=NULL;
342+
ReleaseExternalFD();
315343
}
316344
}
317345

‎src/backend/access/transam/xlog.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,7 @@ static const char *const xlogSourceNames[] = {"any", "archive", "pg_wal", "strea
774774
* openLogFile is -1 or a kernel FD for an open log file segment.
775775
* openLogSegNo identifies the segment. These variables are only used to
776776
* write the XLOG, and so will normally refer to the active segment.
777+
* Note: call Reserve/ReleaseExternalFD to track consumption of this FD.
777778
*/
778779
staticintopenLogFile=-1;
779780
staticXLogSegNoopenLogSegNo=0;
@@ -785,6 +786,9 @@ static XLogSegNo openLogSegNo = 0;
785786
* will be just past that page. readLen indicates how much of the current
786787
* page has been read into readBuf, and readSource indicates where we got
787788
* the currently open file from.
789+
* Note: we could use Reserve/ReleaseExternalFD to track consumption of
790+
* this FD too; but it doesn't currently seem worthwhile, since the XLOG is
791+
* not read by general-purpose sessions.
788792
*/
789793
staticintreadFile=-1;
790794
staticXLogSegNoreadSegNo=0;
@@ -2447,6 +2451,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible)
24472451
/* create/use new log file */
24482452
use_existent= true;
24492453
openLogFile=XLogFileInit(openLogSegNo,&use_existent, true);
2454+
ReserveExternalFD();
24502455
}
24512456

24522457
/* Make sure we have the current logfile open */
@@ -2455,6 +2460,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible)
24552460
XLByteToPrevSeg(LogwrtResult.Write,openLogSegNo,
24562461
wal_segment_size);
24572462
openLogFile=XLogFileOpen(openLogSegNo);
2463+
ReserveExternalFD();
24582464
}
24592465

24602466
/* Add current page to the set of pending pages-to-dump */
@@ -2605,6 +2611,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible)
26052611
XLByteToPrevSeg(LogwrtResult.Write,openLogSegNo,
26062612
wal_segment_size);
26072613
openLogFile=XLogFileOpen(openLogSegNo);
2614+
ReserveExternalFD();
26082615
}
26092616

26102617
issue_xlog_fsync(openLogFile,openLogSegNo);
@@ -3811,6 +3818,7 @@ XLogFileClose(void)
38113818
}
38123819

38133820
openLogFile=-1;
3821+
ReleaseExternalFD();
38143822
}
38153823

38163824
/*
@@ -5224,6 +5232,11 @@ BootStrapXLOG(void)
52245232
use_existent= false;
52255233
openLogFile=XLogFileInit(1,&use_existent, false);
52265234

5235+
/*
5236+
* We needn't bother with Reserve/ReleaseExternalFD here, since we'll
5237+
* close the file again in a moment.
5238+
*/
5239+
52275240
/* Write the first page with the initial record */
52285241
errno=0;
52295242
pgstat_report_wait_start(WAIT_EVENT_WAL_BOOTSTRAP_WRITE);

‎src/backend/postmaster/pgstat.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,9 @@ pgstat_init(void)
602602

603603
pg_freeaddrinfo_all(hints.ai_family,addrs);
604604

605+
/* Now that we have a long-lived socket, tell fd.c about it. */
606+
ReserveExternalFD();
607+
605608
return;
606609

607610
startup_failed:

‎src/backend/postmaster/postmaster.c

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2554,9 +2554,14 @@ ClosePostmasterPorts(bool am_syslogger)
25542554
(errcode_for_file_access(),
25552555
errmsg_internal("could not close postmaster death monitoring pipe in child process: %m")));
25562556
postmaster_alive_fds[POSTMASTER_FD_OWN]=-1;
2557+
/* Notify fd.c that we released one pipe FD. */
2558+
ReleaseExternalFD();
25572559
#endif
25582560

2559-
/* Close the listen sockets */
2561+
/*
2562+
* Close the postmaster's listen sockets. These aren't tracked by fd.c,
2563+
* so we don't call ReleaseExternalFD() here.
2564+
*/
25602565
for (i=0;i<MAXLISTEN;i++)
25612566
{
25622567
if (ListenSocket[i]!=PGINVALID_SOCKET)
@@ -2566,7 +2571,10 @@ ClosePostmasterPorts(bool am_syslogger)
25662571
}
25672572
}
25682573

2569-
/* If using syslogger, close the read side of the pipe */
2574+
/*
2575+
* If using syslogger, close the read side of the pipe. We don't bother
2576+
* tracking this in fd.c, either.
2577+
*/
25702578
if (!am_syslogger)
25712579
{
25722580
#ifndefWIN32
@@ -4279,6 +4287,9 @@ BackendInitialize(Port *port)
42794287
/* Save port etc. for ps status */
42804288
MyProcPort=port;
42814289

4290+
/* Tell fd.c about the long-lived FD associated with the port */
4291+
ReserveExternalFD();
4292+
42824293
/*
42834294
* PreAuthDelay is a debugging aid for investigating problems in the
42844295
* authentication cycle: it can be set in postgresql.conf to allow time to
@@ -6442,6 +6453,20 @@ restore_backend_variables(BackendParameters *param, Port *port)
64426453
strlcpy(pkglib_path,param->pkglib_path,MAXPGPATH);
64436454

64446455
strlcpy(ExtraOptions,param->ExtraOptions,MAXPGPATH);
6456+
6457+
/*
6458+
* We need to restore fd.c's counts of externally-opened FDs; to avoid
6459+
* confusion, be sure to do this after restoring max_safe_fds. (Note:
6460+
* BackendInitialize will handle this for port->sock.)
6461+
*/
6462+
#ifndefWIN32
6463+
if (postmaster_alive_fds[0] >=0)
6464+
ReserveExternalFD();
6465+
if (postmaster_alive_fds[1] >=0)
6466+
ReserveExternalFD();
6467+
#endif
6468+
if (pgStatSock!=PGINVALID_SOCKET)
6469+
ReserveExternalFD();
64456470
}
64466471

64476472

@@ -6584,6 +6609,10 @@ InitPostmasterDeathWatchHandle(void)
65846609
(errcode_for_file_access(),
65856610
errmsg_internal("could not create pipe to monitor postmaster death: %m")));
65866611

6612+
/* Notify fd.c that we've eaten two FDs for the pipe. */
6613+
ReserveExternalFD();
6614+
ReserveExternalFD();
6615+
65876616
/*
65886617
* Set O_NONBLOCK to allow testing for the fd's presence with a read()
65896618
* call.

‎src/backend/postmaster/syslogger.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,11 @@ SysLogger_Start(void)
562562
* This means the postmaster must continue to hold the read end of the
563563
* pipe open, so we can pass it down to the reincarnated syslogger. This
564564
* is a bit klugy but we have little choice.
565+
*
566+
* Also note that we don't bother counting the pipe FDs by calling
567+
* Reserve/ReleaseExternalFD. There's no real need to account for them
568+
* accurately in the postmaster or syslogger process, and both ends of the
569+
* pipe will wind up closed in all other postmaster children.
565570
*/
566571
#ifndefWIN32
567572
if (syslogPipe[0]<0)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp