From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
cc: Amit Kapila <amit.kapila16@gmail.com>, Noah Misch <noah@leadboat.com>,
        Amit Khandekar <amitdkhan.pg@gmail.com>,
        Andres Freund <andres@anarazel.de>,
        Juan =?iso-8859-1?Q?Jos=E9_Santamar=EDa?= Flecha
 <juanjo.santamaria@gmail.com>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
        Robert Haas <robertmhaas@gmail.com>,
        Thomas Munro <thomas.munro@gmail.com>,
        Tomas Vondra <tomas.vondra@2ndquadrant.com>
Subject: Re: logical decoding : exceeded maxAllocatedDescs for .spill files
In-reply-to: <20200109144429.GA27927@alvherre.pgsql>
References: <20200109144429.GA27927@alvherre.pgsql>
Comments: In-reply-to Alvaro Herrera <alvherre@2ndquadrant.com>
	message dated "Thu, 09 Jan 2020 11:44:29 -0300"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <6484.1578583522.1@sss.pgh.pa.us>
Date: Thu, 09 Jan 2020 10:25:22 -0500
Message-ID: <6485.1578583522@sss.pgh.pa.us>
Precedence: bulk

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Hmm, so why not revert the test only in the back branches, given that
> it's not so onerous in master?

I grow tired of repeating myself, but: it's purely accidental that this
test passes in master for the existing set of buildfarm members.

If I have to do so to prove my point, I will set up a buildfarm member
that uses USE_NAMED_POSIX_SEMAPHORES, and then insist that the patch
cope with that.

But the real issue is that the test is abusing max_files_per_process
to do something it was never intended for.  What it was intended for,
and works well at, is to constrain the total FD consumption of a
collection of backends.  It doesn't work well to constrain the maximum
allocatedDescs consumption, because there's too much variability in
our demand for other FDs.  If we feel that we should have a test that
is constraining that, then we need to invent some other mechanism to
do it with.  If we're not willing to invent an appropriate mechanism
to support the test, then we should drop the test, because a
half-baked test is worse than none.

An appropriate mechanism, perhaps, would be some way to constrain
max_safe_fds directly, without any platform- or environment-dependent
effects in the way.  It could be as simple as


	/*
	 * Take off the FDs reserved for system() etc.
	 */
	max_safe_fds -= NUM_RESERVED_FDS;

+	/*
+	 * Apply debugging limit, if defined.
+	 */
+#ifdef MAX_SAFE_FDS_LIMIT
+	max_safe_fds = Min(max_safe_fds, MAX_SAFE_FDS_LIMIT);
+#endif
+
	/*
	 * Make sure we still have enough to get by.
	 */

and then somebody who was concerned about this could run a buildfarm
member with "-DMAX_SAFE_FDS_LIMIT=10" or so.

			regards, tom lane