Re: DSA failed to allocate memory

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Dongming Liu <ldming101(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: DSA failed to allocate memory
Date: 2023-02-20 04:52:48
Message-ID: CA+hUKGJhi3NsF+XyRx6gJu0Y5n7+TE5qMBMzAcDsOKZL7Nw2cw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 20, 2023 at 11:02 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> Yeah. I think the analysis looks good, but I'll do some testing next
> week with the aim of getting it committed. Looks like it now needs
> Meson changes, but I'll look after that as my penance.

Here's an updated version that I'm testing...

Changes to the main patch:

* Adjust a few comments
* pgindent
* Explained a bit more in the commit message

I'm wondering about this bit in rebin_segment():

+ if (segment_map->header == NULL)
+ return;

Why would we be rebinning an uninitialised/unused segment? Does
something in your DSA-client code (I guess you have an extension?) hit
this case? The tests certainly don't; I'm not sure how the case could
be reached.

Changes to the test:

* Update copyright year
* Size -> size_t
* pgindent
* Add Meson glue
* Re-alphabetise the makefile
* Make sure we get BGWH_STOPPED while waiting for bgworkers to exit
* Background worker main function return type is fixed (void)
* results[1] -> results[FLEXIBLE_ARRAY_MEMBER]
* getpid() -> MyProcPid

I wonder if this code would be easier to understand, but not
materially less efficient, if we re-binned eagerly when allocating
too, so the bin is always correct/optimal. Checking fpm_largest()
again after allocating should be cheap, I guess (it just reads a
member variable that we already paid the cost of maintaining). We
don't really seem to amortise much, we just transfer the rebinning
work to the next caller to consider the segment. I haven't tried out
that theory though.

Attachment Content-Type Size
v3-0001-Re-bin-segment-when-memory-pages-are-freed.patch application/x-patch 4.6 KB
v3-0002-Add-a-test-module-to-exercise-dsa.c.patch application/x-patch 19.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2023-02-20 05:21:39 Re: Add WAL read stats to pg_stat_wal
Previous Message Amit Kapila 2023-02-20 04:14:48 Re: Support logical replication of DDLs