Horton

Re: logical decoding : exceeded maxAllocatedDescs for .spill files

Amit Khandekar <amitdkhan.pg@gmail.com>

View thread
From: Amit Khandekar <amitdkhan.pg@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Alvaro Herrera from 2ndQuadrant <alvherre@alvh.no-ip.org>, 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>, Tom Lane <tgl@sss.pgh.pa.us>, Tomas Vondra <tomas.vondra@2ndquadrant.com>
Date: 2019-11-27T08:46:16Z
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

On Tue, 26 Nov 2019 at 12:10, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Nov 26, 2019 at 11:19 AM Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
> >
> > On Tue, 26 Nov 2019 at 10:49, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > >
> > > So, what is the next step here?  How about if we somehow check whether
> > > the file exists before doing unlink, say by using stat?
> > But the thing is, the behaviour is so much in a grey area, that we
> > cannot reliably say for instance that when stat() says there is no
> > such file, there is indeed no such file,
> >
>
> Why so?
This was just an example. What I meant was, we are really not sure of
the behaviour of file operations when the file is in this state.
unlink() returning "Permission denied" when called twice is itself
weird enough.

>
> > and if we re-create  the same
> > file when it is still open, it is always going to open a new file,
> > etc.
> >
>
> Yeah, or maybe even if we don't create with the same name, there is
> always be some dangling file which again doesn't sound like a good
> thing.

Right.

>
> > > If that doesn't work, I think we might need to go in the direction of tracking
> > > file handles in some way, so that they can be closed during an abort.
> > Yeah, that is one way. I am still working on different approaches.
> > WIll get back with proposals.
> >
>
> Fair enough.  See, if you can also consider an approach that is local
> to ReorderBuffer module wherein we can track those handles in
> ReorderBufferTxn or some other place local to that module.

What I found was : We do attempt to close the opened vfds in the
PG_CATCH block. In ReorderBufferCommit(), ReorderBufferIterTXNFinish
is called both in PG_TRY and PG_CATCH. This closes all the opened
vfds. But the issue is : if the ereport() occurs inside
ReorderBufferIterTXNInit(), then iterstate is still NULL. So in
PG_CATCH, ReorderBufferIterTXNFinish() is not called, so the vfds in
state->entries[] remain open.

We can have &iterstate passed to ReorderBufferIterTXNInit() as another
argument, and initialize it first thing inside the function. This way,
it will never be NULL. But need to be careful about the possibility of
having a iterstate in a half-cooked state, so cleanup might use some
uninitialized handles. Will work on it. At least, we can make sure the
iterstate->entries handle doesn't have junk values.


-- 
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company