Re: logical decoding : exceeded maxAllocatedDescs for .spill files

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>, Alvaro Herrera from 2ndQuadrant <alvherre@alvh.no-ip.org>, Amit Khandekar <amitdkhan.pg@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2019-09-12T18:31:37Z
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 →
  1. When a TAP file has non-zero exit status, retain temporary directories.

  2. Fix running out of file descriptors for spill files.

  3. Track statistics for spilling of changes from ReorderBuffer.

  4. Handle ReadFile() EOF correctly on Windows.

  5. Add logical_decoding_work_mem to limit ReorderBuffer memory usage.

  6. Generational memory allocator

  7. Support retaining data dirs on successful TAP tests

Hi,

On 2019-09-12 09:41:02 -0400, Robert Haas wrote:
> On Thu, Sep 12, 2019 at 5:31 AM Tomas Vondra
> <tomas.vondra@2ndquadrant.com> wrote:
> > I don't see how the current API could do that transparently - it does
> > track the files, but the user only gets a file descriptor. With just a
> > file descriptor, how could the code know to do reopen/seek when it's going
> > just through the regular fopen/fclose?
> >
> > Anyway, I agree we need to do something, to fix this corner case (many
> > serialized in-progress transactions). ISTM we have two options - either do
> > something in the context of reorderbuffer.c, or extend the transient file
> > API somehow. I'd say the second option is the right thing going forward,
> > because it does allow doing it transparently and without leaking details
> > about maxAllocatedDescs etc. There are two issues, though - it does
> > require changes / extensions to the API, and it's not backpatchable.
> >
> > So maybe we should start with the localized fix in reorderbuffer, and I
> > agree tracking offset seems reasonable.
> 
> We've already got code that knows how to track this sort of thing.
> You just need to go through the File abstraction (PathNameOpenFile or
> PathNameOpenFilePerm or OpenTemporaryFile) rather than using the
> functions that deal directly with fds (OpenTransientFile,
> BasicOpenFile, etc.).  It seems like it would be better to reuse the
> existing VFD layer than to invent a whole new one specific to logical
> replication.

Yea, I agree that that is the right fix.

Greetings,

Andres Freund