Re: Proposal: Limitations of palloc inside checkpointer

Hi, Xuneng!

On Thu, Jun 26, 2025 at 4:31 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> Patch v7 modifies CompactCheckpointerRequestQueue() to process requests incrementally in batches of CKPT_REQ_BATCH_SIZE (10,000), similar to the approach used in AbsorbSyncRequests(). This limits memory usage from O(num_requests) to O(batch_size) for both hash tables and skip arrays.
>
>
> - Hash table memory bounded by batch size regardless of total queue size
>
> - Skip array allocation limited to batch size instead of max_requests
>
> - Prevents potential OOM conditions with very large request queues
>
>
> Trade-offs
>
> Cross-batch duplicate detection: The incremental approach won't detect duplicates spanning batch boundaries. This limitation seems acceptable since:
>
>    - The main issue need to be addressed is preventing memory allocation failures
>
>    - Remaining duplicates are still handled by the RememberSyncRequest() function in the sync subsystem
>
>    - The purpose of this function is to make some rooms for new requests not remove all duplicates.
>
>
> Lock holding Duration
>
> Andres pointed out[1] that compacting a very large queue takes considerable time, and holding the exclusive lock for an extended period makes it much more likely that backends will have to perform syncs themselves - which is exactly what CompactCheckpointerRequestQueue() is trying to avoid in the first place. However, releasing the lock between batches would introduce race conditions that would make the design much more complex. Given that the primary goal of this patch is to avoid large memory allocations, I keep the lock held for the whole function for simplicity now.
>
> [1] https://postgrespro.com/list/id/bjno37ickfafixkqmd2lcyopsajnuig5mm4rg6tn2ackpqyiba@w3sjfo3usuos

I've reviewed the v7 of patch.  I can note following.
1) The patch makes CompactCheckpointerRequestQueue() process queue in
batches, but doesn't release the lock between batches.  The algorithm
becomes more complex, and it holds the lock for the same (or probably
longer) time.  We trade possibility to miss some duplicates to less
memory consumption.   However, with MAX_CHECKPOINT_REQUESTS limit,
maximal memory consumption shouldn't be too high anyway.
2) Even if we manage to release the lock between the batches then we
still need some additional coordination to prevent two processed doing
CompactCheckpointerRequestQueue() simultaneously.
3) Another problem of releasing the lock is that in spite of
AbsorbSyncRequests() there is no work to do while not holding the
lock.  Releasing the lock and immediately taking it back have a little
use: other processes have almost no chance to grab the lock.

In the token of all of above, I think it's not reasonable to do
CompactCheckpointerRequestQueue() in batches.  Sorry for the
confusion.

Regarding the gap filling, I've got from [1] that the requests order
matters.  Then gap filling strategy is impossible or too complex.  The
point is withdrawn.  I think v6 is basically good enough.

The v8 is attached.  It's basically the same as v6, but has revised
commit message and comments.

The patch for stable branches is also attached: it just limits the
maximum size of the checkpointer requests queue.

Links.
1. https://www.postgresql.org/message-id/CABPTF7XSSecQ-k7k9cQJsA3ACHmCVwdoRfv4DxOMom4cNQL%3D5Q%40mail.gmail.com

------
Regards,
Alexander Korotkov
Supabase