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: 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-08-17T12:58:47Z
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 Sat, Aug 15, 2020 at 3:32 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, Aug 13, 2020 at 6:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Aug 13, 2020 at 12:08 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Fri, Aug 7, 2020 at 2:04 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >
> > > > On Thu, Aug 6, 2020 at 2:43 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > >
> > > ..
> > > > > This patch's functionality can be independently verified by SQL APIs
> > > >
> > > > Your changes look fine to me.
> > > >
> > >
> > > I have pushed that patch last week and attached are the remaining
> > > patches. I have made a few changes in the next patch
> > > 0001-Extend-the-BufFile-interface.patch and have some comments on it
> > > which are as below:
> > >
> >
> > Few more comments on the latest patches:
> > v48-0002-Add-support-for-streaming-to-built-in-replicatio
> > 1. It appears to me that we don't remove the temporary folders created
> > by the apply worker. So, we have folders like
> > pgsql_tmp15324.0.sharedfileset in base/pgsql_tmp directory even when
> > the apply worker exits. I think we can remove these by calling
> > PathNameDeleteTemporaryDir in SharedFileSetUnregister while removing
> > the fileset from registered filesetlist.
>
> I think we need to call SharedFileSetDeleteAll(input_fileset), from
> SharedFileSetUnregister, so that all the directories created for this
> fileset are removed
>
> > 2.
> > +typedef struct SubXactInfo
> > +{
> > + TransactionId xid; /* XID of the subxact */
> > + int fileno; /* file number in the buffile */
> > + off_t offset; /* offset in the file */
> > +} SubXactInfo;
> > +
> > +static uint32 nsubxacts = 0;
> > +static uint32 nsubxacts_max = 0;
> > +static SubXactInfo *subxacts = NULL;
> > +static TransactionId subxact_last = InvalidTransactionId;
> >
> > Will it be better if we move all the subxact related variables (like
> > nsubxacts, nsubxacts_max and subxact_last) inside SubXactInfo struct
> > as all the information anyway is related to sub-transactions?
>
> I have moved them all to a structure.
>
> > 3.
> > + /*
> > + * If there is no subtransaction then nothing to do,  but if already have
> > + * subxact file then delete that.
> > + */
> >
> > extra space before 'but' in the above sentence is not required.
>
> Fixed
>
> > v48-0001-Extend-the-BufFile-interface
> > 4.
> > - * SharedFileSets can also be used by backends when the temporary files need
> > - * to be opened/closed multiple times and the underlying files need to survive
> > + * SharedFileSets can be used by backends when the temporary files need to be
> > + * opened/closed multiple times and the underlying files need to survive
> >   * across transactions.
> >   *
> >
> > No need of 'also' in the above sentence.
>
> Fixed
>

In last patch v49-0001, there is one issue,  Basically, I have called
BufFileFlush in all the cases.  But, ideally, we can not call this if
the underlying files are deleted/truncated because those files/blocks
might not exist now.  So I think if the truncate position is within
the same buffer we just need to adjust the buffer,  otherwise we just
need to set the currFile and currOffset to the absolute number and set
the pos and nbytes 0.  Attached patch fixes this issue.

+ errmsg("could not truncate file \"%s\": %m",
+ FilePathName(file->files[i]))));
+ curOffset = offset;
+ }
+ }
+
+ /* Otherwise, must reposition buffer, so flush any dirty data */
+ BufFileFlush(file);
+

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