Re: Proposal: Limitations of palloc inside checkpointer

On Thu, Aug 7, 2025 at 4:01 PM Xuneng Zhou wrote: > On Thu, Aug 7, 2025 at 3:16 PM Xuneng Zhou wrote: > > > > Hi, Tom! > > > > Thanks for catching this. > > > > On Thu, Aug 7, 2025 at 2:58 AM Tom Lane wrote: > > > > > > Alexander Korotkov writes: > > > > I'm going to push this if no objections.

Re: Proposal: Limitations of palloc inside checkpointer

Hi, On Thu, Aug 7, 2025 at 3:16 PM Xuneng Zhou wrote: > > Hi, Tom! > > Thanks for catching this. > > On Thu, Aug 7, 2025 at 2:58 AM Tom Lane wrote: > > > > Alexander Korotkov writes: > > > I'm going to push this if no objections. > > > > I looked at these patches while preparing release notes,

Re: Proposal: Limitations of palloc inside checkpointer

Hi, Tom! Thanks for catching this. On Thu, Aug 7, 2025 at 2:58 AM Tom Lane wrote: > > Alexander Korotkov writes: > > I'm going to push this if no objections. > > I looked at these patches while preparing release notes, and > found an oversight. CheckpointerShmemInit does > > Checkpoint

Re: Proposal: Limitations of palloc inside checkpointer

Alexander Korotkov writes: > I'm going to push this if no objections. I looked at these patches while preparing release notes, and found an oversight. CheckpointerShmemInit does CheckpointerShmem->max_requests = Min(NBuffers, MAX_CHECKPOINT_REQUESTS); but CheckpointerShmemSize still d

Re: Proposal: Limitations of palloc inside checkpointer

On Fri, Jul 25, 2025 at 3:23 PM Xuneng Zhou wrote: > > Thanks for reviewing and feedback! > > > 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 ch

Re: Proposal: Limitations of palloc inside checkpointer

Hi, Alexander Thanks for reviewing and feedback! > 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. LGTM. Best, Xuneng

Re: Proposal: Limitations of palloc inside checkpointer

Hi, Xuneng! On Thu, Jun 26, 2025 at 4:31 PM Xuneng Zhou 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_req

Re: Proposal: Limitations of palloc inside checkpointer

Hi, 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

Re: Proposal: Limitations of palloc inside checkpointer

Hi, > > 3) Fill gaps by pulling from the tail instead of rewriting the whole > queue? > > > > I misunderstood at first—this is a generally helpful optimization. > > I'll integrate it into the current patch. > > Great, thank you. > I dug deeper into the “fill gaps from the tail” optimization and

Re: Proposal: Limitations of palloc inside checkpointer

Hi, Thanks for the feedback! > I think it would be good start point to use the same batch size of > CompactCheckpointerRequestQueue() and AbsorbSyncRequests() > So we’ll keep both batch processing and the request cap in place for now. > > > Right, but another point is to avoid lengthy holding o

Re: Proposal: Limitations of palloc inside checkpointer

Hi all, Sorry—I forgot to Cc on my previous message. Resending here so they’re on the thread: On Wed, Jun 4, 2025 at 11:07 AM Xuneng Zhou wrote: > > Hi Alexander, > > Thanks again for the feedback! > > 1) Batch-processing CompactCheckpointerRequestQueue() and > AbsorbSyncRequests()? > > After s

Re: Proposal: Limitations of palloc inside checkpointer

On Tue, Jun 3, 2025 at 1:16 PM Xuneng Zhou wrote: > Thanks a lot for reviewing! > > > I have few notes about that: > > 1) Should we make CompactCheckpointerRequestQueue() process the queue > > of checkpoint requests in smaller parts for the same reason we do this > > in AbsorbSyncRequests()? That

Re: Proposal: Limitations of palloc inside checkpointer

Hi Alexander, Thanks a lot for reviewing! > I have few notes about that: > 1) Should we make CompactCheckpointerRequestQueue() process the queue > of checkpoint requests in smaller parts for the same reason we do this > in AbsorbSyncRequests()? That would require significant redesign of > the alg

Re: Proposal: Limitations of palloc inside checkpointer

Hi, Xuneng Zhou! On Tue, Apr 15, 2025 at 7:02 AM Xuneng Zhou wrote: > I've moved this entry to the July CommitFest. The CI reported an unused > variable warning in patch v5, so I've addressed it by removing the unused > one. Sorry for not catching the issue locally. Thank you for your work on

Re: Proposal: Limitations of palloc inside checkpointer

Hi, I've moved this entry to the July CommitFest. The CI reported an unused variable warning in patch v5, so I've addressed it by removing the unused one. Sorry for not catching the issue locally. Xuneng Zhou 于2025年4月10日周四 23:43写道： > Hi, > > I’ve updated and rebased Maxim's patch version 4 with

Re: Proposal: Limitations of palloc inside checkpointer

Hi, I’ve updated and rebased Maxim's patch version 4 with optimizations like ring buffer and capped max_requests proposed by Heikki. These are the summary of Changes in v5: • Replaced the linear array model in AbsorbSyncRequests() with a bounded ring buffer to avoid large memmove() operations wh

Re: Proposal: Limitations of palloc inside checkpointer

On 12/03/2025 13:00, Maxim Orlov wrote: On Wed, 12 Mar 2025 at 10:27, Xuneng Zhou > wrote: The patch itself looks ok to me. I'm curious about the trade-offs between this incremental approach and the alternative of using palloc_extended() with the MCXT_ALLOC_HUGE fla

Re: Proposal: Limitations of palloc inside checkpointer

On Wed, 12 Mar 2025 at 10:27, Xuneng Zhou wrote: > Hi, > The patch itself looks ok to me. I'm curious about the trade-offs between > this incremental approach and the alternative of > using palloc_extended() with the MCXT_ALLOC_HUGE flag. The approach of > splitting the requests into fixed-size s

Re: Proposal: Limitations of palloc inside checkpointer

Hi, The patch itself looks ok to me. I'm curious about the trade-offs between this incremental approach and the alternative of using palloc_extended() with the MCXT_ALLOC_HUGE flag. The approach of splitting the requests into fixed-size slices avoids OOM failures or process termination by the OOM

Re: Proposal: Limitations of palloc inside checkpointer

I think I figured it out. Here is v4. If the number of requests is less than 1 GB, the algorithm stays the same as before. If we need to process more, we will do it incrementally with slices of 1 GB. Best regards, Maxim Orlov. v4-0001-Process-sync-requests-incrementally-in-AbsorbSync.patch Desc

Re: Proposal: Limitations of palloc inside checkpointer

Here is an alternative solution. We can limit the number of processed requests to fit in a 1Gb memory chunk for each pass. Obviously, we left some requests in the queue to be processed in the next call. We can overcome this by using multi-step processing: estimating the number of steps in the begin

Re: Proposal: Limitations of palloc inside checkpointer

After done some testing, I found a bug in the patch. If more requests were pushed while we release the lock, num_requests could not be set to zero. Here is a fixed version. -- Best regards, Maxim Orlov. v2-0001-AbsorbSyncRequests-incrementally-instead-of-doing.patch Description: Binary data

Re: Proposal: Limitations of palloc inside checkpointer

I tried to implement the idea (4). This is the patch. But, there is a problem. See, when we release lock and call RememberSyncRequest() and acquire it again, CompactCheckpointerRequestQueue() may be called and the state of the request array may be changed. Of course, we can recheck num_requests af

Re: Proposal: Limitations of palloc inside checkpointer

On Wed, 26 Feb 2025 at 11:54, Andres Freund wrote: > > 4) Do compaction incrementally, instead of doing it for all requests at > once. Yeah, good idea! I completely forgot about that. Thanks! -- Best regards, Maxim Orlov.

Re: Proposal: Limitations of palloc inside checkpointer

Hi, On 2025-02-26 11:46:45 +0300, Maxim Orlov wrote: > On Tue, 25 Feb 2025 at 22:44, Ekaterina Sokolova > wrote: > > > Hi, hackers! > > > > Historically, the checkpointer process use palloc() into > > AbsorbSyncRequests() function. Therefore, the checkpointer does not > > expect to receive a req

Re: Proposal: Limitations of palloc inside checkpointer

On Tue, 25 Feb 2025 at 22:44, Ekaterina Sokolova wrote: > Hi, hackers! > > Historically, the checkpointer process use palloc() into > AbsorbSyncRequests() function. Therefore, the checkpointer does not > expect to receive a request larger than 1 GB. Yeah. And the most unpleasant thing is it won

Proposal: Limitations of palloc inside checkpointer

Hi, hackers! Historically, the checkpointer process use palloc() into AbsorbSyncRequests() function. Therefore, the checkpointer does not expect to receive a request larger than 1 GB. We encountered a case where the database went into recovery state, after applying all wal, the checkpointer