Re: logical decoding : exceeded maxAllocatedDescs for .spill files

Tom Lane <tgl@sss.pgh.pa.us>

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 José Santamaría 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>

Date: 2020-01-09T15:25:22Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

When a TAP file has non-zero exit status, retain temporary directories.
- 048c7ccd7d6d 9.6.17 landed
- d8efc5900f7c 10.12 landed
- 887657d183fc 11.7 landed
- 78a26c3edd85 12.2 landed
- bf989aaf3561 13.0 landed
Fix running out of file descriptors for spill files.
- 1ad47e8757bb 9.4.26 landed
- a6f4f407ada0 9.5.21 landed
- 27b5f48c79f7 10.12 landed
- 3e3a79735235 11.7 landed
- f8a6d8e71b17 12.2 landed
- d20703805383 13.0 landed
- ba5b4e506489 9.6.17 landed
Track statistics for spilling of changes from ReorderBuffer.
- 9290ad198b15 13.0 cited
Handle ReadFile() EOF correctly on Windows.
- 2189f49c420f 12.2 landed
- 6969deeb8d39 13.0 landed
Add logical_decoding_work_mem to limit ReorderBuffer memory usage.
- cec2edfa7859 13.0 cited
Generational memory allocator
- a4ccc1cef5a0 11.0 cited
Support retaining data dirs on successful TAP tests
- 90627cf98a8e 11.0 cited

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