On Wed, Mar 13, 2019 at 11:41 AM Michael Banck <michael.ba...@credativ.de> wrote:
> Am Mittwoch, den 13.03.2019, 18:31 +0900 schrieb Michael Paquier: > > On Wed, Mar 13, 2019 at 10:08:33AM +0100, Fabien COELHO wrote: > > > I'm not sure of the punctuation logic on the help line: the first > sentence > > > does not end with a ".". I could not find an instance of this style in > other > > > help on pg commands. I'd suggest "check data checksums (default)" > would work > > > around and be more in line with other commands help. > > > > Good idea, let's do that. > > > > > I slowed down pg_checksums by adding a 0.1s sleep when scanning a new > file, > > > then started a "pg_checksums --enable" on a stopped cluster, then > started > > > the cluster while the enabling was in progress, then connected and > updated > > > data. > > > > Well, yes, don't do that. You can get into the same class of problems > > while running pg_rewind, pg_basebackup or even pg_resetwal once the > > initial control file check is done for each one of these tools. > > > > > I do not think it is a good thing that two commands can write to the > data > > > directory at the same time, really. > > > > We don't prevent either a pg_resetwal and a pg_basebackup to run in > > parallel. That would be... Interesting. > > But does pg_basebackup actually change the primary's data directory? I > don't think so, so that does not seem to be a problem. > > pg_rewind and pg_resetwal are (TTBOMK) pretty quick operations, while > pg_checksums can potentially run for hours, so I see the point of taking > extra care here. > > On the other hand, two pg_checksums running in parallel also seem not > much of a problem as the cluster is offline anyway. > > What is much more of a footgun is one DBA starting pg_checksums --enable > on a 1TB cluster, then going for lunch, and then the other DBA wondering > why the DB is down and starting the instance again. > > We read the control file on pg_checksums' startup, so once pg_checksums > finishs it'll write the old checkpoint LSN into pg_control (along with > the updated checksum version). This is pilot error, but I think we > should try to guard against it. > > I propose we re-read the control file for the enable case after we > finished operating on all files and (i) check the instance is still > offline and (ii) update the checksums version from there. That should be > a small but worthwhile change that could be done anyway. > In (i) you need to also check that is' not offline *again*. Somebody could start *and* stop the database while pg_checksums is running. But that should hopefully be enough to check the time field? Another option would be to add a new feature which reliably blocks an > instance from starting up due to maintenance - either a new control file > field, some message in postmaster.pid (like "pg_checksums maintenance in > progress") that would prevent pg_ctl or postgres/postmaster from > starting up like 'FATAL: bogus data in lock file "postmaster.pid": > "pg_checksums in progress' or some other trigger file. > Instead of overloading yet another thing on postmaster.pid, it might be better to just have a separate file that if it exists, blocks startup with a message defined as the content of that file? > It > > could be possible to reach a state where the control file has > > checksums enabled and some blocks are not correctly synced, still you > > would notice rather quickly if the server is in an incorrect state at > > the follow-up startup. > > Would you? I think I'm with Fabien on this one and it seems worthwhile > to run fsync_pgdata() before and after update_controlfile() - the second > one should be really quick anyway. > > Also, I suggest to maybe add a notice in verbose mode that we are > syncing the data directory - otherwise the user might wonder what's > going on at 100% done, though I haven't seen a large delay in my tests > so far. > Seems like a good idea -- there certainly could be a substantial delay there depending on data size and underlying storage. -- Magnus Hagander Me: https://www.hagander.net/ <http://www.hagander.net/> Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>