Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions
Date
Msg-id CAA4eK1Kdmi6VVguKEHV6Ho2isCPVFdQtt0WLsK10fiuE59_0Yw@mail.gmail.com
Whole thread Raw
In response to Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions  (vignesh C <vignesh21@gmail.com>)
Responses Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions  (Dilip Kumar <dilipbalaut@gmail.com>)
List pgsql-hackers
On Wed, Nov 6, 2019 at 11:33 AM vignesh C <vignesh21@gmail.com> wrote:
>
> I have made one change to the configuration file in
> contrib/test_decoding directory, with that the coverage seems to be
> fine. I have seen that the coverage is almost like the code before
> applying the patch. I have attached the test change and the coverage
> report for reference. Coverage report includes the core logical work
> memory files for base code and by applying
> 0001-Add-logical_decoding_work_mem-to-limit-ReorderBuffer and
> 0002-Track-statistics-for-spilling patches.
>

Thanks,  I have incorporated your test changes and modified the two
patches.  Please see attached.

Changes:
---------------
1. In guc.c, we should include reorderbuffer.h, not logical.h as we
define logical_decoding_work_mem in earlier.

2.
+ *   To limit the amount of memory used by decoded changes, we track memory
+ *   used at the reorder buffer level (i.e. total amount of memory), and for
+ *   each toplevel transaction. When the total amount of used memory exceeds
+ *   the limit, the toplevel transaction consuming the most memory is then
+ *   serialized to disk.

In the above comments, removed 'toplevel' as we track memory usage for
both toplevel and subtransactions.

3. There were still a few mentions of streaming which I have removed.

4. In the docs, the type for stats spill_* was integer whereas it
should be bigint.

5.
+UpdateSpillStats(LogicalDecodingContext *ctx)
+{
+ ReorderBuffer *rb = ctx->reorder;
+
+ SpinLockAcquire(&MyWalSnd->mutex);
+
+ MyWalSnd->spillTxns = rb->spillTxns;
+ MyWalSnd->spillCount = rb->spillCount;
+ MyWalSnd->spillBytes = rb->spillBytes;
+
+ elog(WARNING, "UpdateSpillStats: updating stats %p %ld %ld %ld",
+ rb, rb->spillTxns, rb->spillCount, rb->spillBytes);

Changed the above elog to DEBUG1 as otherwise it was getting printed
very frequently.  I think we can make it DEBUG2 if we want.

6. There was an extra space in rules.out due to which test was
failing.  I have fixed it.

What do you think?

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Attachment

pgsql-hackers by date:

Previous
From: Sergei Fedorov
Date:
Subject: Re: [Patch proposal] libpq portal support
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: [proposal] recovery_target "latest"