Re: Refactoring the checkpointer's fsync request queue

Thomas Munro <thomas.munro@enterprisedb.com>

From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>, Pg Hackers <pgsql-hackers@postgresql.org>, Shawn Debnath <sdn@amazon.com>
Date: 2018-11-16T21:53:20Z
Lists: pgsql-hackers
On Thu, Nov 15, 2018 at 5:09 AM Dmitry Dolgov <9erthalion6@gmail.com> wrote:
> While testing this patch with frequent checkpoints I've stumbled upon an
> interesting error, that happened already after I finished one test:
>
> TRAP: FailedAssertion("!(rc > 0)", File: "checkpointer.c", Line: 574)

Thanks for testing!  Yeah, that's:

+       rc = WaitEventSetWait(wes, cur_timeout * 1000, &event, 1, 0);
+       Assert(rc > 0);

I got confused about the API.  If there is a timeout, you get rc == 0,
but I think I was expecting rc == 1, event.event == WL_TIMEOUT.  Oops.
I will fix that when I post a new experimental version that uses the
bgworker as discussed, and we can try to figure out if that design
will fly.

-- 
Thomas Munro
http://www.enterprisedb.com


Commits

  1. Fix bugs in mdsyncfiletag().

  2. Refactor the fsync queue for wider use.

  3. Don't forget about failed fsync() requests.

  4. PANIC on fsync() failure.

  5. Move LockClauseStrength, LockWaitPolicy into new file nodes/lockoptions.h.

  6. Add new file for checkpointer.c

  7. Split work of bgwriter between 2 processes: bgwriter and checkpointer.

  8. Install infrastructure for shared-memory free space map. Doesn't actually