Re: Using condition variables to wait for checkpoints
On Thu, Mar 14, 2019 at 11:05 AM Thomas Munro wrote: > I renamed the CVs because the names I had used before broke the > convention that variables named ckpt_* are protected by ckpt_lck, and > pushed. Erm... this made successful checkpoints slightly faster but failed checkpoints infinitely slower. It would help if we woke up CV waiters in the error path too. Patch attached. -- Thomas Munro https://enterprisedb.com From b6ad979bf2280eee1b0fbbe4c13eaefeb82743c8 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Fri, 5 Apr 2019 21:43:29 +1300 Subject: [PATCH] Wake up interested backends when a checkpoint fails. Commit c6c9474a switched to condition variables instead of sleep loops to notify backends of checkpoint start and stop, but forgot to broadcast in case of checkpoint failure. Author: Thomas Munro Discussion: --- src/backend/postmaster/checkpointer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index d303ce3679..a1e04239ad 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -279,6 +279,8 @@ CheckpointerMain(void) CheckpointerShmem->ckpt_done = CheckpointerShmem->ckpt_started; SpinLockRelease(>ckpt_lck); + ConditionVariableBroadcast(>done_cv); + ckpt_active = false; } -- 2.21.0
Re: Using condition variables to wait for checkpoints
On Thu, Mar 14, 2019 at 1:15 AM Robert Haas wrote: > On Tue, Mar 12, 2019 at 7:12 PM Andres Freund wrote: > > Having useful infrastructure is sure cool. > > Yay! +1 I renamed the CVs because the names I had used before broke the convention that variables named ckpt_* are protected by ckpt_lck, and pushed. There are some other things like this in the tree (grepping for poll/pg_usleep loops finds examples in xlog.c, standby.c, ...). That might be worth looking into. -- Thomas Munro https://enterprisedb.com
Re: Using condition variables to wait for checkpoints
On Tue, Mar 12, 2019 at 7:12 PM Andres Freund wrote: > Having useful infrastructure is sure cool. Yay! -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Using condition variables to wait for checkpoints
Hi, On 2019-03-13 11:56:19 +1300, Thomas Munro wrote: > A user complained about CREATE DATABASE taking > 200ms even with fsync > set to off. Andres pointed out that that'd be the clunky poll/sleep > loops in checkpointer.c. > > Here's a draft patch to use condition variables instead. > > Unpatched: > > postgres=# checkpoint; > CHECKPOINT > Time: 101.848 ms > > Patched: > > postgres=# checkpoint; > CHECKPOINT > Time: 1.851 ms Neat. That's with tiny shmem though, I bet? > + > + CheckpointDone > + Waiting for a checkpoint to complete. > + > + > + CheckpointStart > + Waiting for a checkpoint to start. > + Not sure I like these much, but I can't quite ome up with something meaningfully better. Looks good to me. Having useful infrastructure is sure cool. - Andres
