Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Dilip Kumar <dilipbalaut@gmail.com>
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Tighten the concurrent abort check during decoding.
- 2ce353fc1902 14.0 landed
-
Improve hash_create()'s API for some added robustness.
- b3817f5f7746 14.0 landed
-
Use HASH_BLOBS for xidhash.
- a1b8aa1e4eec 14.0 landed
-
Fix initialization of RelationSyncEntry for streaming transactions.
- 69bd60672af6 14.0 landed
-
Remove unused function declaration in logicalproto.h.
- ddd5f6d2609b 14.0 landed
-
Add additional tests to test streaming of in-progress transactions.
- 58b5ae9d62bd 14.0 landed
-
Fix inline marking introduced in commit 464824323e.
- ac15b499f7f9 14.0 landed
-
Add support for streaming to built-in logical replication.
- 464824323e57 14.0 landed
-
Fix the SharedFileSetUnregister API.
- 4ab77697f67a 14.0 landed
-
Fix comment in procarray.c
- 77c7267c37f7 14.0 cited
-
Suppress compiler warning in non-cassert builds.
- e942af7b8261 14.0 cited
-
Extend the BufFile interface.
- 808e13b282ef 14.0 landed
-
Mark a few logical decoding related variables with PGDLLIMPORT.
- b48cac3b10a0 14.0 landed
-
Implement streaming mode in ReorderBuffer.
- 7259736a6e5b 14.0 landed
-
Extend the logical decoding output plugin API with stream methods.
- 45fdc9738b36 14.0 landed
-
WAL Log invalidations at command end with wal_level=logical.
- c55040ccd017 14.0 landed
-
Immediately WAL-log subtransaction and top-level XID association.
- 0bead9af484c 14.0 landed
-
Allow logical replication to transfer data in binary format.
- 9de77b545313 14.0 cited
-
Only superuser can set sslcert/sslkey in postgres_fdw user mappings
- cebf9d6e6ee1 13.0 cited
-
Track statistics for spilling of changes from ReorderBuffer.
- 9290ad198b15 13.0 landed
-
Add logical_decoding_work_mem to limit ReorderBuffer memory usage.
- cec2edfa7859 13.0 landed
-
logical decoding: process ASSIGNMENT during snapshot build
- bac2fae05c77 13.0 cited
-
Emit invalidations to standby for transactions without xid.
- c6ff84b06a68 9.6.0 cited
On Mon, Jul 13, 2020 at 4:00 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> 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.
Done this way In the latest patch set.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com