On 3 April 2016 at 21:32, Simon Riggs <si...@2ndquadrant.com> wrote: > On 14 March 2016 at 17:46, David Steele <da...@pgmasters.net> wrote: > >> On 2/24/16 12:40 AM, Michael Paquier wrote: >> >> This has the merit to be clear, thanks for the input. Whatever the >>> approach taken at the end we have two candidates: >>> - Extend XLogInsert() with an extra argument for flags (Andres) >>> - Introduce XLogInsertExtended with this extra argument and let >>> XLogInsert() in peace (Robert and I). >>> Actually, I lied, there was still something I could do for this >>> thread: attached are two patches implementing both approaches as >>> respectively a-1 and a-2. Patch b is the set of logs I used for the >>> tests to show with a low checkpoint_timeout that checkpoints are >>> getting correctly skipped on an idle system. >>> >> >> Unfortunately neither A nor B apply anymore. >> >> However, since the patches can still be read through I wonder if Robert >> or Andres would care to opine on whether A or B is better now that they can >> see the full implementation? >> >> For my 2c I'm happy with XLogInsertExtended() since it seems to be a rare >> use case where flags are required. This can always be refactored in the >> future if/when the use of flags spreads. >> > > XLogInsertExtended() is the one I would commit, if. >
I'm not very excited about this patch. Too much code for so little benefit and fragile too. I'm not even sure what definition of "meaningful progress" is useful. If we commit this, a similar bug could be filed for many similar WAL record types. Since we zero out WAL files specifically to allow them to be compressed in the archive, this patch saves a few bytes in the archive. Seems like we could fake up some WAL files if needed for restore with an external tool, if we really care. Since we agree it wouldn't be backpatched, its pretty much saying we don't care. A promising approach would be to just skip logging the RUNNING_XACTS record if there are no xacts running, which is the case we care about here (no progress => no xacts). HS startup can only happen at a checkpoint, so if there is no WAL file there is no checkpoint, so we don't need the RUNNING_XACTS record to be written. That's a short patch. -- Simon Riggs http://www.2ndQuadrant.com/ <http://www.2ndquadrant.com/> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
