Thread: Using condition variables to wait for checkpoints
Hello hackers, 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 -- Thomas Munro https://enterprisedb.com
Attachment
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? > + <row> > + <entry><literal>CheckpointDone</literal></entry> > + <entry>Waiting for a checkpoint to complete.</entry> > + </row> > + <row> > + <entry><literal>CheckpointStart</literal></entry> > + <entry>Waiting for a checkpoint to start.</entry> > + </row> 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
On Tue, Mar 12, 2019 at 7:12 PM Andres Freund <andres@anarazel.de> wrote: > Having useful infrastructure is sure cool. Yay! -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Mar 14, 2019 at 1:15 AM Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Mar 12, 2019 at 7:12 PM Andres Freund <andres@anarazel.de> 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
On Thu, Mar 14, 2019 at 11:05 AM Thomas Munro <thomas.munro@gmail.com> 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