From: Andres Freund <andres@anarazel.de>
To: Shawn Debnath <sdn@amazon.com>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Refactoring the checkpointer's fsync request queue
Date: Wed, 20 Feb 2019 15:50:26 -0800
Message-ID: <20190220235026.2pxdcmwtijijax32@alap3.anarazel.de> (raw)
In-Reply-To: <20190220232739.GA8280@f01898859afd.ant.amazon.com>
References: <CA+q6zcV=p=1G2Z8itJ9=tAn=b8ha3pO4RQCMt9YXOEutRV33YQ@mail.gmail.com>
<CAEepm=2qBbQXGeekhCY+W8m=fy-+eU_FAUzRCSPFKuZ2rKG1LQ@mail.gmail.com>
<CAEepm=1uqZLcTqLz_+egZRYotnTvzDK-jOGZdN8QhL=7nixY5Q@mail.gmail.com>
<CAEepm=1mp+7PPY+sKJ4cvs6hba0NnWeed00oJ=EWry0gUgnJ_A@mail.gmail.com>
<20190131055938.GA52540@f01898859afd.ant.amazon.com>
<20190213025843.GA52283@f01898859afd.ant.amazon.com>
<CAEepm=0_0r4H1LLbG42XbVHHLoDnRh3JWGtj-rnvdWa4TV77Vw@mail.gmail.com>
<20190216024502.iluiqfniwa7eolb4@alap3.anarazel.de>
<20190216193905.GA53174@f01898859afd.ant.amazon.com>
<20190220232739.GA8280@f01898859afd.ant.amazon.com>
Hi,
Thanks for the update!
On 2019-02-20 15:27:40 -0800, Shawn Debnath wrote:
> As promised, here's a patch that addresses the points discussed by
> Andres and Thomas at FOSDEM. As a result of how we want checkpointer to
> track what files to fsync, the pending ops table now integrates the
> forknum and segno as part of the hash key eliminating the need for the
> bitmapsets, or vectors from the previous iterations. We re-construct the
> pathnames from the RelFileNode, ForkNumber and SegmentNumber and use
> PathNameOpenFile to get the file descriptor to use for fsync.
I still object to exposing segment numbers to smgr and above. I think
that's an implementation detail of the various storage managers, and we
shouldn't expose smgr.c further than it already is.
> Apart from that, this patch moves the system for requesting and
> processing fsyncs out of md.c into smgr.c, allowing us to call on smgr
> component specific callbacks to retrieve metadata like relation and
> segment paths. This allows smgr components to maintain how relfilenodes,
> forks and segments map to specific files without exposing this knowledge
> to smgr. It redefines smgrsync() behavior to be closer to that of
> smgrimmedsysnc(), i.e., if a regular sync is required for a particular
> file, enqueue it in locally or forward it to checkpointer.
> smgrimmedsync() retains the existing behavior and fsyncs the file right
> away. The processing of fsync requests has been moved from mdsync() to a
> new ProcessFsyncRequests() function.
I think that's also wrong, imo fsyncs are storage detail, and should be
relegated to *below* md.c. That's not to say the code should live in
md.c, but the issuing of such calls shouldn't be part of smgr.c -
consider e.g. developing a storage engine living in non volatile ram.
Greetings,
Andres Freund
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: andres@anarazel.de, sdn@amazon.com, thomas.munro@enterprisedb.com, 9erthalion6@gmail.com, robertmhaas@gmail.com
Subject: Re: Refactoring the checkpointer's fsync request queue
In-Reply-To: <20190220235026.2pxdcmwtijijax32@alap3.anarazel.de>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox