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-06-28T15:25:21Z
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 Wed, Jun 24, 2020 at 4:04 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> Comments on other patches:
> =========================

Replying to the pending comments.

> 6.
> In v29-0002-Issue-individual-invalidations-with-wal_level-lo,
> xact_desc_invalidations seems to be a subset of
> standby_desc_invalidations, can we have a common code for them?

Done

> 7.
> I think we can avoid sending v29-0007-Track-statistics-for-streaming
> this each time.  We can do this after the main patch is complete.
> Also, we might need to change how and where these stats will be
> tracked.  See the related discussion [1].

Removed

> 8. In v29-0005-Implement-streaming-mode-in-ReorderBuffer,
>   * Return oldest transaction in reorderbuffer
> @@ -863,6 +909,9 @@ ReorderBufferAssignChild(ReorderBuffer *rb,
> TransactionId xid,
>   /* set the reference to top-level transaction */
>   subtxn->toptxn = txn;
>
> + /* set the reference to toplevel transaction */
> + subtxn->toptxn = txn;
> +
>
> There is a double initialization of subtxn->toptxn.  You need to
> remove this line from 0005 patch as we have now added it in an earlier
> patch.

Done

> 9.  I think you forgot to update the patch to execute invalidations in
> Abort case or I might be missing something.  I don't see any changes
> in ReorderBufferAbort. You have agreed in one of the emails above [2]
> about handling the same.

Done, check 0005

> 10. In v29-0008-Add-support-for-streaming-to-built-in-replicatio,
>  apply_handle_stream_commit(StringInfo s)
>  {
>  ..
>  + /*
>  + * send feedback to upstream
>  + *
>  + * XXX Probably should send a valid LSN. But which one?
>  + */
>  + send_feedback(InvalidXLogRecPtr, false, false);
>  ..
>  }
>
> I have given a comment on this code that we don't need this feedback
> and you mentioned on June 02 [3] that you will think on it and let me
> know your opinion but I don't see a response from you yet.  Can you
> get back to me regarding this point?

Yeah, I have analyzed this and this seems we don't need this.  Because
like non-streaming mode here also sending feedback mechanisms shall be
the same.  I don't see any reason for sending extra feedback on
commit.

> 11. Add some comments as to why we have used Shared BufFile interface
> instead of Temp BufFile interface?

Done

> 12. In v29-0013-Change-buffile-interface-required-for-streaming,
> + * Initialize a space for temporary files that can be opened other backends.
>
> /opened other backends/opened for access by other backends

Done

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