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
