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: Jeff Janes <jeff.janes@gmail.com>, Erik Rijkers <er@xs4all.nl>, Kuntal Ghosh <kuntalghosh.2007@gmail.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Michael Paquier <michael@paquier.xyz>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-09-02T10:11:25Z
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.

Attachments

On Wed, Sep 2, 2020 at 10:55 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Sep 1, 2020 at 8:33 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > I have fixed all the comments except
> ..
> > 3. +# Change the local values of the extra columns on the subscriber,
> > +# update publisher, and check that subscriber retains the expected
> > +# values
> > +$node_subscriber->safe_psql('postgres', "UPDATE test_tab SET c =
> > 'epoch'::timestamptz + 987654321 * interval '1s'");
> > +$node_publisher->safe_psql('postgres', "UPDATE test_tab SET b = md5(a::text)");
> > +
> > +wait_for_caught_up($node_publisher, $appname);
> > +
> > +$result =
> > +  $node_subscriber->safe_psql('postgres', "SELECT count(*),
> > count(extract(epoch from c) = 987654321), count(d = 999) FROM
> > test_tab");
> > +is($result, qq(3334|3334|3334), 'check extra columns contain locally
> > changed data');
> >
> > Again, how this test is relevant to streaming mode?
> >
>
> I think we can keep this test in one of the newly added tests say in
> 015_stream_simple.pl to ensure that after streaming transaction, the
> non-streaming one behaves expectedly. So we can change the comment as
> "Change the local values of the extra columns on the subscriber,
> update publisher, and check that subscriber retains the expected
> values. This is to ensure that non-streaming transactions behave
> properly after a streaming transaction."
>
> We can remove this test from the other two places
> 016_stream_subxact.pl and 020_stream_binary.pl.
>
> > 4. Apart from the above, I think we should think of minimizing the
> > test cases which can be committed with the base patch. We can later
> > add more tests.
> >
>
> We can combine the tests in 015_stream_simple.pl and
> 020_stream_binary.pl as I can't see a good reason to keep them
> separate. Then, I think we can keep only this part with the main patch
> and extract other tests into a separate patch. Basically, we can
> commit the basic tests with the main patch and then keep the advanced
> tests separately. I am afraid that there are some tests that don't add
> much value so we can review them separately.

Fixed

> One minor comment for option 'streaming = on', spacing-wise it should
> be consistent in all the tests.
>
> Similarly, we can combine 017_stream_ddl.pl and 021_stream_schema.pl
> as both contains similar tests. As per the above suggestion, this will
> be in a separate patch though.
>
> If you agree with the above suggestions then kindly make these
> adjustments and send the updated patch.

Done that way.

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