Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Amit Kapila <amit.kapila16@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 Wed, Jul 22, 2020 at 10:20 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, Jul 22, 2020 at 9:18 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Jul 20, 2020 at 6:46 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > There was one warning in release mode in the last version in 0004 so
> > > attaching a new version.
> > >
> >
> > Today, I was reviewing patch
> > v38-0001-WAL-Log-invalidations-at-command-end-with-wal_le and found a
> > small problem with it.
> >
> > + /*
> > + * Execute the invalidations for xid-less transactions,
> > + * otherwise, accumulate them so that they can be processed at
> > + * the commit time.
> > + */
> > + if (!ctx->fast_forward)
> > + {
> > + if (TransactionIdIsValid(xid))
> > + {
> > + ReorderBufferAddInvalidations(reorder, xid, buf->origptr,
> > + invals->nmsgs, invals->msgs);
> > + ReorderBufferXidSetCatalogChanges(ctx->reorder, xid,
> > + buf->origptr);
> > + }
> >
> > I think we need to set ReorderBufferXidSetCatalogChanges even when
> > ctx->fast-forward is true because we are dependent on that flag for
> > snapshot build (see SnapBuildCommitTxn). We are already doing the
> > same way in DecodeCommit where even though we skip adding
> > invalidations for fast-forward cases but we do set the flag to
> > indicate that this txn has catalog changes. Is there any reason to do
> > things differently here?
>
> I think it is wrong, we should set the
> ReorderBufferXidSetCatalogChanges, even if it is in fast-forward mode.
>
Thanks for the change. I have one more minor comment in the patch
0001-WAL-Log-invalidations-at-command-end-with-wal_le.
/*
+ * Invalidations logged with wal_level=logical.
+ */
+typedef struct xl_xact_invalidations
+{
+ int nmsgs; /* number of shared inval msgs */
+ SharedInvalidationMessage msgs[FLEXIBLE_ARRAY_MEMBER];
+} xl_xact_invalidations;
I see that we already have a structure xl_xact_invals in the code
which has the same members, so I think it is better to use that
instead of defining a new one.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com