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

Dilip Kumar <dilipbalaut@gmail.com>

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-06-16T14:19:17Z
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 Tue, Jun 9, 2020 at 3:04 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Jun 8, 2020 at 11:53 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > I think one of the usages we still need is in ReorderBufferForget
> > because it can be called when we skip processing the txn.  See the
> > comments in DecodeCommit where we call this function.  If I am
> > correct, we need to probably collect all invalidations in
> > ReorderBufferTxn as we are collecting tuplecids and use them here.  We
> > can do the same during processing of XLOG_XACT_INVALIDATIONS.
> >
>
> One more point related to this is that after this patch series, we
> need to consider executing all invalidation during transaction abort.
> Because it is possible that due to memory overflow, we have processed
> some of the messages which also contain a few XACT_INVALIDATION
> messages, so to avoid cache pollution, we need to execute all of them
> in abort.  We also do the similar thing in Rollback/Rollback To
> Savepoint, see AtEOXact_Inval and AtEOSubXact_Inval.

I have analyzed this further and I think there is some problem with
that. If Instead of keeping the invalidation as an individual change,
if we try to combine them in ReorderBufferTxn's invalidation then what
happens if the (sub)transaction is aborted.  Basically, in this case,
we will end up executing all those invalidations for those we never
polluted the cache if we never try to stream it.  So this will affect
the normal case where we haven't streamed the transaction because
every time we have executed the invalidation logged by transaction
those are aborted.  One way is we develop the list at the
sub-transaction level and just before sending the transaction (on
commit) combine all the (sub) transaction's invalidation list.  But,
I think since we already have the invalidation in the commit message
then there is no point in adding this complexity.
But, my main worry is about the streaming transaction, the problems are
- Immediately on the arrival of individual invalidation, we can not
directly add to the top-level transaction's invalidation list because
later if the transaction aborted before we stream (or we directly
stream on commit) then we will get an unnecessarily long list of
invalidation which is done by aborted subtransaction.
- If we keep collecting in the individual subtransaction's
ReorderBufferTxn->invalidations,  then the problem is when to merge
it?  I think it is a good idea to merge them all as soon as we try to
stream it/or on commit?  So since this solution of combining the (sub)
transaction's invalidation is required for the streaming case we can
use it as common solution whether it streams due to the memory
overflow or due to the commit.

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