Re: logical decoding : exceeded maxAllocatedDescs for .spill files

On Tue, 26 Nov 2019 at 12:10, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Nov 26, 2019 at 11:19 AM Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> wrote:
> >
> > On Tue, 26 Nov 2019 at 10:49, Amit Kapila <amit(dot)kapila16(at)gmail(dot)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