From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Noah Misch <noah(at)leadboat(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Why is src/test/modules/committs/t/002_standby.pl flaky? |
Date: | 2024-01-05 17:04:32 |
Message-ID: | CA+TgmoaUZ-JusZD8KwZ4ZnCygNM_e3hZRvn4qsvDaKG3shv1jg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Nov 9, 2023 at 10:32 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> Here is a new attempt to fix this mess. Disclaimer: this based
> entirely on reading the manual and vicariously hacking a computer I
> don't have via CI.
I'd first like to congratulate this thread on reaching its second
birthday. The CommitFest entry hasn't quite made it to the two year
mark yet - expect that in another month or so - but thread itself is
over the line.
Regarding 0001, I don't know if we really need SH_RAW_FREE. You can
just define your own SH_FREE implementation in userspace. That doesn't
work for SH_RAW_ALLOCATOR because there's code in simplehash.h that
knows about memory contexts apart from the actual definition of
SH_ALLOCATE - e.g. we include a MemoryContext pointer in SH_TYPE, and
in the signature of SH_CREATE. But SH_FREE doesn't seem to have any
similar issues. Maybe it's still worth doing for convenience -- I
haven't thought about that very hard -- but it doesn't seem to be
required in the same way that SH_RAW_ALLOCATOR was.
I wonder whether we really want 0002. It seems like a pretty
significant behavior change -- now everybody using simplehash has to
worry about whether failure cases are possible. And maybe there's some
performance overhead. And most of the changes are restricted to the
SH_RAW_ALLOCATOR case, but the changes to SH_GROW are not. And making
this contingent on SH_RAW_ALLOCATOR doesn't seem principled.
I kind of wonder whether trying to handle OOM here is the wrong
direction to go. What if we just bail out hard if we can't insert into
the hash table? I think that we don't expect the hash table to ever be
very large (right?) and we don't install these kinds of defenses
everywhere that OOM on a small memory allocation is a possibility (or
at least I don't think we do). I'm actually sort of unclear about why
you're trying to force this to use raw malloc/free instead of
palloc/pfree. Do we need to use this on the frontend side? Do we need
it on the backend side prior to the memory context infrastructure
being up?
--
Robert Haas
EDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2024-01-05 17:08:39 | Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs |
Previous Message | Nathan Bossart | 2024-01-05 17:04:27 | Re: add AVX2 support to simd.h |