Horton

Re: logical decoding : exceeded maxAllocatedDescs for .spill files

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Amit Khandekar <amitdkhan.pg@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-12-16T11:22:00Z
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 Mon, Dec 16, 2019 at 3:26 PM Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
>
> On Sat, 14 Dec 2019 at 11:59, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > I have also made minor changes related to below code in patch:
> > - else if (readBytes != sizeof(ReorderBufferDiskChange))
> > +
> > + file->curOffset += readBytes;
> > +
> > + if (readBytes !=
> > sizeof(ReorderBufferDiskChange))
> >
> > Why the size is added before the error check?
> The logic was : even though it's an error that the readBytes does not
> match the expected size, the file read is successful so update the vfd
> offset as early as possible. In our case, this might not matter much,
> but who knows, in the future, in the exception block (say, in
> ReorderBufferIterTXNFinish, someone assumes that the file offset is
> correct and does something with that, then we will get in trouble,
> although I agree that it's very unlikely.
>

I am not sure if there is any such need, but even if it is there, I
think updating after a *short* read (read less than expected) doesn't
seem like a good idea because there is clearly some problem with the
read call.  Also, in the case below that case where we read the actual
change data, the offset is updated after the check of *short* read.  I
don't see any advantage in such an inconsistency.  I still feel it is
better to update the offset after all error checks.

>
> > and see if you can run perltidy for the test file.
> Hmm, I tried perltidy, and it seems to mostly add a space after ( and
> a space before ) if there's already; so "('postgres'," is replaced by
> "(<space> 'postgres',". And this is going to be inconsistent with
> other places. And it replaces tab with spaces. Do you think we should
> try perltidy, or have we before been using this tool for the tap tests
> ?
>

See text in src/test/perl/README (Note that all tests and test tools
should have perltidy run on them before patches are submitted, using
perltidy - profile=src/tools/pgindent/perltidyrc).  It is recommended
to use perltidy.

Now, if it is making the added code inconsistent with nearby code,
then I suggest to leave it.


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com