Hi,

On 2024-11-26 11:35:56 -0500, Tom Lane wrote:
> Andres Freund <and...@anarazel.de> writes:
> > I think it's rather confusing to claim that epoll_create1() failed when we
> > didn't even call it.
> > Why are we misattributing the failure to a system call that we didn't make?
> 
> I think the idea was that this mechanism is equivalent to an EMFILE
> limit.  But if you feel a need to make a distinction, this seems fine:
> 
> >                 elog(ERROR, "AcquireExternalFD, for epoll_create1, failed: 
> > %m");
> 
> You should probably check all of 3d475515a, because I think
> I applied the same idea in more than one place.

Yea, there's another equivalent message for kqueue a few lines below.

The commit also added uses of AcquireExternalFD() to dblink and postgres_fdw (
since migrated to libpq-be-fe-helpers.h), but I don't think those need
improving in the same way. They never made it sound like it was a syscall
itself failing.

Greetings,

Andres Freund
>From 6ba3876a01c89884fa718a407bf7cb3094bd9106 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 26 Nov 2024 12:20:59 -0500
Subject: [PATCH v1] Distinguish between AcquireExternalFD and epoll_create1 /
 kqueue failing

The error messages in CreateWaitEventSet() made it hard to know whether the
syscall or AcquireExternalFD() failed. This is particularly relevant because
AcquireExternalFD() imposes a lower limit than what would cause syscalls fail
with EMFILE.

I did not change the message in libpqsrv_connect_prepare(), which is the one
other use of AcquireExternalFD() in our codebase, as the error message already
is less ambiguous.

Discussion: https://postgr.es/m/xjjx7r4xa7beixuu4qtkdhnwdbchrrpo3gaeb3jsbinvvdiat5@cwjw55mna5of
---
 src/backend/storage/ipc/latch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c
index 608eb66abed..78d43f07d03 100644
--- a/src/backend/storage/ipc/latch.c
+++ b/src/backend/storage/ipc/latch.c
@@ -813,7 +813,7 @@ CreateWaitEventSet(ResourceOwner resowner, int nevents)
 	if (!AcquireExternalFD())
 	{
 		/* treat this as though epoll_create1 itself returned EMFILE */
-		elog(ERROR, "epoll_create1 failed: %m");
+		elog(ERROR, "AcquireExternalFD, for epoll_create1, failed: %m");
 	}
 	set->epoll_fd = epoll_create1(EPOLL_CLOEXEC);
 	if (set->epoll_fd < 0)
@@ -825,7 +825,7 @@ CreateWaitEventSet(ResourceOwner resowner, int nevents)
 	if (!AcquireExternalFD())
 	{
 		/* treat this as though kqueue itself returned EMFILE */
-		elog(ERROR, "kqueue failed: %m");
+		elog(ERROR, "AcquireExternalFD, for kqueue, failed: %m");
 	}
 	set->kqueue_fd = kqueue();
 	if (set->kqueue_fd < 0)
-- 
2.45.2.746.g06e570c0df.dirty

Reply via email to