On Wed, Nov 2, 2016 at 10:23 AM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Tue, Nov 1, 2016 at 10:31 PM, Robert Haas <robertmh...@gmail.com> wrote: >> IMHO, your rewrite of this patch was a bit heavy-handed. > > OK... Sorry for that. > >> I haven't >> scrutinized the code here so maybe it was a big improvement, and if so >> fine, but if not it's better to collaborate with the author than to >> take over. > > While reviewing the code, that has finished by being a large rewrite, > and that was more understandable than a review looking at all the > small tweaks and things I have been through while reading it. I have > also experimented a couple of ideas with the patch that I added, so at > the end it proves to be a gain for everybody. I think that the last > patch is an improvement, if you want to make your own opinion on the > matter looking at the differences between both patches would be the > most direct way to go. > If my understanding is correct regarding this feature, last two patches completely break the fundamental idea of wal consistency check feature. I mentioned this in my last reply as well that we've to use some flag to indicate whether an image should be restored during replay or not. Otherwise, XLogReadBufferForRedoExtended will always restore the image skipping the usual redo operation. What's happening now is the following: 1. If wal_consistency is on, include backup block image with the wal record. 2. During replay, XLogReadBufferForRedoExtended always restores the backup block image in local buffer since XLogRecHasBlockImage is true for each block. 3. In checkConsistency, you compare the local buffer with the backup block image from the wal record. It'll always be consistent. This feature aims to validate whether wal replay operation is happening correctly or not. To achieve that aim, we should not alter the wal replay operation itself.
Rest of the suggestions are well-taken. I'll update the patch accordingly. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
