Horton

Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

Dilip Kumar <dilipbalaut@gmail.com>

View thread
From: Dilip Kumar <dilipbalaut@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2019-11-07T10:19:51Z
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. Tighten the concurrent abort check during decoding.

  2. Improve hash_create()'s API for some added robustness.

  3. Use HASH_BLOBS for xidhash.

  4. Fix initialization of RelationSyncEntry for streaming transactions.

  5. Remove unused function declaration in logicalproto.h.

  6. Add additional tests to test streaming of in-progress transactions.

  7. Fix inline marking introduced in commit 464824323e.

  8. Add support for streaming to built-in logical replication.

  9. Fix the SharedFileSetUnregister API.

  10. Fix comment in procarray.c

  11. Suppress compiler warning in non-cassert builds.

  12. Extend the BufFile interface.

  13. Mark a few logical decoding related variables with PGDLLIMPORT.

  14. Implement streaming mode in ReorderBuffer.

  15. Extend the logical decoding output plugin API with stream methods.

  16. WAL Log invalidations at command end with wal_level=logical.

  17. Immediately WAL-log subtransaction and top-level XID association.

  18. Allow logical replication to transfer data in binary format.

  19. Only superuser can set sslcert/sslkey in postgres_fdw user mappings

  20. Track statistics for spilling of changes from ReorderBuffer.

  21. Add logical_decoding_work_mem to limit ReorderBuffer memory usage.

  22. logical decoding: process ASSIGNMENT during snapshot build

  23. Emit invalidations to standby for transactions without xid.

On Thu, Nov 7, 2019 at 3:19 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Nov 6, 2019 at 11:33 AM vignesh C <vignesh21@gmail.com> wrote:
> >
> > I have made one change to the configuration file in
> > contrib/test_decoding directory, with that the coverage seems to be
> > fine. I have seen that the coverage is almost like the code before
> > applying the patch. I have attached the test change and the coverage
> > report for reference. Coverage report includes the core logical work
> > memory files for base code and by applying
> > 0001-Add-logical_decoding_work_mem-to-limit-ReorderBuffer and
> > 0002-Track-statistics-for-spilling patches.
> >
>
> Thanks,  I have incorporated your test changes and modified the two
> patches.  Please see attached.
>
> Changes:
> ---------------
> 1. In guc.c, we should include reorderbuffer.h, not logical.h as we
> define logical_decoding_work_mem in earlier.
Yeah Right.
>
> 2.
> + *   To limit the amount of memory used by decoded changes, we track memory
> + *   used at the reorder buffer level (i.e. total amount of memory), and for
> + *   each toplevel transaction. When the total amount of used memory exceeds
> + *   the limit, the toplevel transaction consuming the most memory is then
> + *   serialized to disk.
>
> In the above comments, removed 'toplevel' as we track memory usage for
> both toplevel and subtransactions.
Correct.
>
> 3. There were still a few mentions of streaming which I have removed.
>
ok
> 4. In the docs, the type for stats spill_* was integer whereas it
> should be bigint.
ok
>
> 5.
> +UpdateSpillStats(LogicalDecodingContext *ctx)
> +{
> + ReorderBuffer *rb = ctx->reorder;
> +
> + SpinLockAcquire(&MyWalSnd->mutex);
> +
> + MyWalSnd->spillTxns = rb->spillTxns;
> + MyWalSnd->spillCount = rb->spillCount;
> + MyWalSnd->spillBytes = rb->spillBytes;
> +
> + elog(WARNING, "UpdateSpillStats: updating stats %p %ld %ld %ld",
> + rb, rb->spillTxns, rb->spillCount, rb->spillBytes);
>
> Changed the above elog to DEBUG1 as otherwise it was getting printed
> very frequently.  I think we can make it DEBUG2 if we want.
Yeah, it should not be WARNING.
>
> 6. There was an extra space in rules.out due to which test was
> failing.  I have fixed it.
My Bad.  I have induced while separating out the changes for the spilling.

> What do you think?
I have reviewed your changes and looks fine to me.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com