Horton

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

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: 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-07-13T10:30:03Z
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 Mon, Jul 13, 2020 at 3:04 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Jul 13, 2020 at 2:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Jul 13, 2020 at 2:32 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Mon, Jul 13, 2020 at 11:10 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > >
> > > > I think you can refer to commit message as well for that "We however
> > > > must explicitly disable streaming replication during replication slot
> > > > creation, even if the plugin supports it. We don't need to replicate
> > > > the changes accumulated during this phase, and moreover, we don't have
> > > > a replication connection open so we don't have where to send the data
> > > > anyway.".  I don't think this is a good way to hack the streaming flag
> > > > because for SQL API's, we don't have a good reason to disable the
> > > > streaming in this way.  I guess if we had a condition related to
> > > > reaching CONSISTENT snapshot during streaming then we won't need to
> > > > hack the streaming flag in this way.  Once we reach the CONSISTENT
> > > > snapshot state, we come out of the creation of a replication slot (see
> > > > how we use DecodingContextReady to achieve that) phase.  So, I feel we
> > > > should remove the ctx->streaming setting to false and add a CONSISTENT
> > > > snapshot check during streaming unless you have a reason for not doing
> > > > so.
> > >
> > > I was worried about the point that streaming on/off is sent by the
> > > subscriber on START REPLICATION, not on CREATE REPLICATION SLOT, so if
> > > we keep streaming on during create then it may not be right.
> > >
> >
> > Then, how is that used on the publisher-side?  AFAICS, the streaming
> > is enabled based on whether streaming callbacks are provided and we do
> > that in 0003-Extend-the-logical-decoding-output-plugin-API-wi patch.
>
> Basically, first, we enable based on whether we have the callbacks or
> not but later once we get the START REPLICATION command from the
> subscriber then we set it to false if the streaming is not enabled
> from the subscriber side.  You can refer below code in patch 0007.
>
> pgoutput_startup
> {
> parse_output_parameters(ctx->output_plugin_options,
> &data->protocol_version,
> - &data->publication_names);
> + &data->publication_names,
> + &enable_streaming);
> /* Check if we support requested protocol */
> if (data->protocol_version > LOGICALREP_PROTO_VERSION_NUM)
> @@ -222,6 +284,27 @@ pgoutput_startup(LogicalDecodingContext *ctx,
> OutputPluginOptions *opt,
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> errmsg("publication_names parameter missing")));
> + /*
> + * Decide whether to enable streaming. It is disabled by default, in
> + * which case we just update the flag in decoding context. Otherwise
> + * we only allow it with sufficient version of the protocol, and when
> + * the output plugin supports it.
> + */
> + if (!enable_streaming)
> + ctx->streaming = false;
> }
>

Okay, in that case, we can do both enable and disable streaming in
this function itself rather than allow the caller to later modify it.
I suggest similarly we can enable/disable it for SQL API in
pg_decode_startup via output_plugin_options.  This way it will look
consistent for both SQL APIs and for command-based replication.  If we
can do so, then probably adding an Assert for Consistent Snapshot
while performing streaming should be okay.

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