Re: [HACKERS] WAL consistency check facility

2017-02-14 Thread Michael Paquier
On Wed, Feb 15, 2017 at 11:08 AM, Robert Haas wrote: > On Tue, Feb 14, 2017 at 7:12 PM, Tom Lane wrote: >> FWIW, my own habit when creating new PG files is generally to write >> >> * Portions Copyright (c) 1996-2017, PostgreSQL Global Development Group

Re: [HACKERS] WAL consistency check facility

2017-02-14 Thread Robert Haas
On Tue, Feb 14, 2017 at 7:12 PM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Feb 14, 2017 at 5:16 PM, Michael Paquier >> wrote: >>> Just for curiosity: does the moment when the code has been written or >>> committed

Re: [HACKERS] WAL consistency check facility

2017-02-14 Thread Tom Lane
Robert Haas writes: > On Tue, Feb 14, 2017 at 5:16 PM, Michael Paquier > wrote: >> Just for curiosity: does the moment when the code has been written or >> committed counts? It's no big deal seeing how liberal the Postgres >> license is, but this

Re: [HACKERS] WAL consistency check facility

2017-02-14 Thread Robert Haas
On Tue, Feb 14, 2017 at 5:16 PM, Michael Paquier wrote: > On Wed, Feb 15, 2017 at 2:43 AM, Robert Haas wrote: >> On Thu, Feb 9, 2017 at 8:17 PM, Michael Paquier >> wrote: >>> Please find attached a patch with those

Re: [HACKERS] WAL consistency check facility

2017-02-14 Thread Michael Paquier
On Wed, Feb 15, 2017 at 2:43 AM, Robert Haas wrote: > On Thu, Feb 9, 2017 at 8:17 PM, Michael Paquier > wrote: >> Please find attached a patch with those fixes. > > Committed, but I changed the copyright dates to 2016-2017 rather than > just 2017

Re: [HACKERS] WAL consistency check facility

2017-02-14 Thread Robert Haas
On Thu, Feb 9, 2017 at 8:17 PM, Michael Paquier wrote: > Please find attached a patch with those fixes. Committed, but I changed the copyright dates to 2016-2017 rather than just 2017 since surely some of the code was originally written before 2017. Even that might

Re: [HACKERS] WAL consistency check facility

2017-02-09 Thread Michael Paquier
On Thu, Feb 9, 2017 at 5:56 AM, Robert Haas wrote: > On Wed, Feb 8, 2017 at 1:25 AM, Kuntal Ghosh > wrote: >> Thank you Robert for the review. Please find the updated patch in the >> attachment. > > I have committed this patch after fairly

Re: [HACKERS] WAL consistency check facility

2017-02-08 Thread Kuntal Ghosh
On Thu, Feb 9, 2017 at 2:26 AM, Robert Haas wrote: > On Wed, Feb 8, 2017 at 1:25 AM, Kuntal Ghosh > wrote: >> Thank you Robert for the review. Please find the updated patch in the >> attachment. > > I have committed this patch after fairly

Re: [HACKERS] WAL consistency check facility

2017-02-08 Thread Robert Haas
On Wed, Feb 8, 2017 at 1:25 AM, Kuntal Ghosh wrote: > Thank you Robert for the review. Please find the updated patch in the > attachment. I have committed this patch after fairly extensive revisions: * Rewrote the documentation to give some idea what the underlying

Re: [HACKERS] WAL consistency check facility

2017-02-07 Thread Kuntal Ghosh
On Tue, Jan 31, 2017 at 9:36 PM, Robert Haas wrote: > On Thu, Jan 5, 2017 at 12:54 AM, Kuntal Ghosh > wrote: >> I've attached the patch with the modified changes. PFA. > > Can this patch check contrib/bloom? > Added support for generic resource

Re: [HACKERS] WAL consistency check facility

2017-02-07 Thread Robert Haas
On Tue, Feb 7, 2017 at 6:32 AM, Amit Kapila wrote: > On Tue, Jan 31, 2017 at 9:36 PM, Robert Haas wrote: >> >> +if (!HeapTupleHeaderXminFrozen(page_htup)) >> +page_htup->t_infomask |= HEAP_XACT_MASK; >> +else

Re: [HACKERS] WAL consistency check facility

2017-02-07 Thread Amit Kapila
On Tue, Jan 31, 2017 at 9:36 PM, Robert Haas wrote: > > +if (!HeapTupleHeaderXminFrozen(page_htup)) > +page_htup->t_infomask |= HEAP_XACT_MASK; > +else > +page_htup->t_infomask |= HEAP_XMAX_COMMITTED | >

Re: [HACKERS] WAL consistency check facility

2017-02-01 Thread Robert Haas
On Tue, Jan 31, 2017 at 9:31 PM, Michael Paquier wrote: > On Wed, Feb 1, 2017 at 1:06 AM, Robert Haas wrote: >> On Thu, Jan 5, 2017 at 12:54 AM, Kuntal Ghosh >> wrote: >>> I've attached the patch with the modified

Re: [HACKERS] WAL consistency check facility

2017-01-31 Thread Kuntal Ghosh
On Wed, Feb 1, 2017 at 8:01 AM, Michael Paquier wrote: > On Wed, Feb 1, 2017 at 1:06 AM, Robert Haas wrote: >> +/* >> + * Leave if no masking functions defined, this is possible in the case >> + * resource managers generating just

Re: [HACKERS] WAL consistency check facility

2017-01-31 Thread Kuntal Ghosh
On Tue, Jan 31, 2017 at 9:36 PM, Robert Haas wrote: > On Thu, Jan 5, 2017 at 12:54 AM, Kuntal Ghosh > wrote: >> I've attached the patch with the modified changes. PFA. Thanks Robert for taking your time for the review. I'll update the patch

Re: [HACKERS] WAL consistency check facility

2017-01-31 Thread Michael Paquier
On Wed, Feb 1, 2017 at 1:06 AM, Robert Haas wrote: > On Thu, Jan 5, 2017 at 12:54 AM, Kuntal Ghosh > wrote: >> I've attached the patch with the modified changes. PFA. > > Can this patch check contrib/bloom? Only full pages are applied at redo

Re: [HACKERS] WAL consistency check facility

2017-01-31 Thread Robert Haas
On Thu, Jan 5, 2017 at 12:54 AM, Kuntal Ghosh wrote: > I've attached the patch with the modified changes. PFA. Can this patch check contrib/bloom? +/* + * Mask some line pointer bits, particularly those marked as + * used on a master and

Re: [HACKERS] WAL consistency check facility

2017-01-30 Thread Michael Paquier
On Thu, Jan 5, 2017 at 2:54 PM, Kuntal Ghosh wrote: > On Wed, Dec 21, 2016 at 10:53 PM, Robert Haas wrote: > >> On a first read-through of this patch -- I have not studied it in >> detail yet -- this looks pretty good to me. One concern is that

Re: [HACKERS] WAL consistency check facility

2017-01-04 Thread Kuntal Ghosh
On Wed, Dec 21, 2016 at 10:53 PM, Robert Haas wrote: > On a first read-through of this patch -- I have not studied it in > detail yet -- this looks pretty good to me. One concern is that this > patch adds a bit of code to XLogInsert(), which is a very hot piece of > code.

Re: [HACKERS] WAL consistency check facility

2016-12-21 Thread Kuntal Ghosh
On Wed, Dec 21, 2016 at 10:53 PM, Robert Haas wrote: > On Mon, Nov 28, 2016 at 11:31 PM, Michael Paquier > wrote: >> Moved to CF 2017-01, as no committers have showed up yet :( > > Seeing no other volunteers, here I am. > Thanks Robert for

Re: [HACKERS] WAL consistency check facility

2016-12-21 Thread Robert Haas
On Mon, Nov 28, 2016 at 11:31 PM, Michael Paquier wrote: > Moved to CF 2017-01, as no committers have showed up yet :( Seeing no other volunteers, here I am. On a first read-through of this patch -- I have not studied it in detail yet -- this looks pretty good to me.

Re: [HACKERS] WAL consistency check facility

2016-11-28 Thread Michael Paquier
On Wed, Nov 16, 2016 at 2:15 AM, Michael Paquier wrote: > On Tue, Nov 15, 2016 at 7:50 AM, Kuntal Ghosh > wrote: >> I've modified the guc parameter name as wal_consistency_check (little >> hesitant for a participle in suffix :) ). Also,

Re: [HACKERS] WAL consistency check facility

2016-11-15 Thread Michael Paquier
On Tue, Nov 15, 2016 at 7:50 AM, Kuntal Ghosh wrote: > I've modified the guc parameter name as wal_consistency_check (little > hesitant for a participle in suffix :) ). Also, updated the sgml and > variable name accordingly. The changes look good to me. -- Michael

Re: [HACKERS] WAL consistency check facility

2016-11-15 Thread Kuntal Ghosh
On Tue, Nov 15, 2016 at 7:23 PM, Robert Haas wrote: > On Sat, Nov 12, 2016 at 10:06 PM, Peter Eisentraut > wrote: >> On 11/9/16 11:55 PM, Michael Paquier wrote: >>> + >>> + wal_consistency (string) >>> + >>> +

Re: [HACKERS] WAL consistency check facility

2016-11-15 Thread Robert Haas
On Sat, Nov 12, 2016 at 10:06 PM, Peter Eisentraut wrote: > On 11/9/16 11:55 PM, Michael Paquier wrote: >> + >> + wal_consistency (string) >> + >> + wal_consistency configuration >> parameter >> + >> + >> + >> + >>

Re: [HACKERS] WAL consistency check facility

2016-11-12 Thread Michael Paquier
On Sun, Nov 13, 2016 at 12:06 PM, Peter Eisentraut wrote: > Could we name this something like wal_consistency_checking? > > Otherwise it sounds like you use this to select the amount of > consistency in the WAL (similar to, say, wal_level). Or wal_check? Or

Re: [HACKERS] WAL consistency check facility

2016-11-12 Thread Peter Eisentraut
On 11/9/16 11:55 PM, Michael Paquier wrote: > + > + wal_consistency (string) > + > + wal_consistency configuration parameter > + > + > + > + > +This parameter is used to check the consistency of WAL records, i.e, > +whether the WAL

Re: [HACKERS] WAL consistency check facility

2016-11-11 Thread Kuntal Ghosh
On Fri, Nov 11, 2016 at 3:36 AM, Michael Paquier wrote: > On Fri, Nov 11, 2016 at 1:36 AM, Robert Haas wrote: >> So, who are all of the people involved in the effort to produce this >> patch, and what's the right way to attribute credit? > > The

Re: [HACKERS] WAL consistency check facility

2016-11-10 Thread Michael Paquier
On Fri, Nov 11, 2016 at 1:36 AM, Robert Haas wrote: > So, who are all of the people involved in the effort to produce this > patch, and what's the right way to attribute credit? The original idea was from Heikki as he has introduced the idea of doing such checks if you

Re: [HACKERS] WAL consistency check facility

2016-11-10 Thread Robert Haas
On Thu, Nov 10, 2016 at 10:02 AM, Kuntal Ghosh wrote: >> With the patch for BRIN applied, I am able to get installcheck-world >> working with wal_consistency = all and a standby doing the consistency >> checks behind. Adding wal_consistency = all in PostgresNode.pm,

Re: [HACKERS] WAL consistency check facility

2016-11-10 Thread Kuntal Ghosh
On Thu, Nov 10, 2016 at 10:25 AM, Michael Paquier wrote: > On Wed, Nov 9, 2016 at 11:32 PM, Kuntal Ghosh >> Thanks a lot for reviewing the patch. Based on your review, I've attached the >> I've done a fair amount of testing which includes regression tests >> and manual

Re: [HACKERS] WAL consistency check facility

2016-11-09 Thread Michael Paquier
On Wed, Nov 9, 2016 at 11:32 PM, Kuntal Ghosh wrote: > On Fri, Nov 4, 2016 at 1:32 PM, Michael Paquier > wrote: >> Thank you for the new patch. This will be hopefully the last round of >> reviews, we are getting close to something that has

Re: [HACKERS] WAL consistency check facility

2016-11-09 Thread Kuntal Ghosh
On Fri, Nov 4, 2016 at 1:32 PM, Michael Paquier wrote: > Thank you for the new patch. This will be hopefully the last round of > reviews, we are getting close to something that has an acceptable > shape. Thanks a lot for reviewing the patch. Based on your review, I've

Re: [HACKERS] WAL consistency check facility

2016-11-04 Thread Michael Paquier
On Fri, Nov 4, 2016 at 6:02 PM, Michael Paquier wrote: > On Fri, Nov 4, 2016 at 5:02 PM, Michael Paquier > wrote: >> On Thu, Nov 3, 2016 at 11:17 PM, Kuntal Ghosh >> wrote: >>> I've updated the patch for review.

Re: [HACKERS] WAL consistency check facility

2016-11-04 Thread Michael Paquier
On Fri, Nov 4, 2016 at 5:02 PM, Michael Paquier wrote: > On Thu, Nov 3, 2016 at 11:17 PM, Kuntal Ghosh > wrote: >> I've updated the patch for review. > > Thank you for the new patch. This will be hopefully the last round of > reviews, we are

Re: [HACKERS] WAL consistency check facility

2016-11-04 Thread Michael Paquier
On Thu, Nov 3, 2016 at 11:17 PM, Kuntal Ghosh wrote: > I've updated the patch for review. Thank you for the new patch. This will be hopefully the last round of reviews, we are getting close to something that has an acceptable shape. + + + + +

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Michael Paquier
On Fri, Nov 4, 2016 at 4:16 AM, Kuntal Ghosh wrote: > On Thu, Nov 3, 2016 at 8:24 PM, Robert Haas wrote: >> On Wed, Nov 2, 2016 at 10:30 AM, Kuntal Ghosh >> wrote: >>> - Another suggestion was to remove

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Kuntal Ghosh
On Thu, Nov 3, 2016 at 8:24 PM, Robert Haas wrote: > On Wed, Nov 2, 2016 at 10:30 AM, Kuntal Ghosh > wrote: >> - Another suggestion was to remove wal_consistency from PostgresNode.pm >> because small buildfarm machines may suffer on it. Although

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Robert Haas
On Wed, Nov 2, 2016 at 10:30 AM, Kuntal Ghosh wrote: > - Another suggestion was to remove wal_consistency from PostgresNode.pm > because small buildfarm machines may suffer on it. Although I've no > experience in this matter, I would like to be certain that nothings

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Kuntal Ghosh
On Thu, Nov 3, 2016 at 7:47 PM, Kuntal Ghosh wrote: > I've updated the patch for review. > If an inconsistency is found, it'll just log it for now. Once, the patch is finalized, we can change it to FATAL as before. I was making sure that all regression tests should

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Kuntal Ghosh
I've updated the patch for review. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com walconsistency_v12.patch Description: application/download -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Michael Paquier
On Thu, Nov 3, 2016 at 6:48 PM, Kuntal Ghosh wrote: > So, whenever we are required to use bimg_info flag, we should make > sure that has_image > is set. OK, we are taking past each other here. There are two possible patterns: - has_image is set, not apply, meaning

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Kuntal Ghosh
On Thu, Nov 3, 2016 at 2:52 PM, Michael Paquier wrote: > On Thu, Nov 3, 2016 at 6:15 PM, Kuntal Ghosh > wrote: >> Actually, I just verified that bimg_info is not even valid if >> has_image is not set. >> In DecodeXLogRecord, we initialize

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Michael Paquier
On Thu, Nov 3, 2016 at 6:15 PM, Kuntal Ghosh wrote: > Actually, I just verified that bimg_info is not even valid if > has_image is not set. > In DecodeXLogRecord, we initialize bimg_info only when has_image flag > is set. So, keeping them > separate doesn't look a good

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Michael Paquier
On Thu, Nov 3, 2016 at 5:56 PM, Kuntal Ghosh wrote: > I'm not getting why we should introduce a new redo action and return > from the function beforehand. Per my last email, same conclusion from here :) Sorry if I am picky and noisy on many points, I am trying to

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Kuntal Ghosh
On Thu, Nov 3, 2016 at 2:27 PM, Michael Paquier wrote: > On Thu, Nov 3, 2016 at 4:04 PM, Michael Paquier > wrote: >> Wouldn't the definition of a new redo action make sense then? Say >> SKIPPED. None of the existing actions match the

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Michael Paquier
On Thu, Nov 3, 2016 at 4:04 PM, Michael Paquier wrote: > Wouldn't the definition of a new redo action make sense then? Say > SKIPPED. None of the existing actions match the non-apply case. I just took 5 minutes to look at the code and reason about it, and something

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Kuntal Ghosh
On Thu, Nov 3, 2016 at 12:34 PM, Michael Paquier wrote: > On Thu, Nov 3, 2016 at 3:24 PM, Kuntal Ghosh > wrote: >> On Thu, Nov 3, 2016 at 2:35 AM, Michael Paquier >>> -/* If it's a full-page image, restore it. */ >>> -if

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Michael Paquier
On Thu, Nov 3, 2016 at 3:24 PM, Kuntal Ghosh wrote: > On Thu, Nov 3, 2016 at 2:35 AM, Michael Paquier > wrote: >>> - Another suggestion was to remove wal_consistency from PostgresNode.pm >>> because small buildfarm machines may suffer on it.

Re: [HACKERS] WAL consistency check facility

2016-11-03 Thread Kuntal Ghosh
On Thu, Nov 3, 2016 at 2:35 AM, Michael Paquier wrote: >> - Another suggestion was to remove wal_consistency from PostgresNode.pm >> because small buildfarm machines may suffer on it. Although I've no >> experience in this matter, I would like to be certain that

Re: [HACKERS] WAL consistency check facility

2016-11-02 Thread Michael Paquier
On Wed, Nov 2, 2016 at 11:30 PM, Kuntal Ghosh wrote: > On Wed, Nov 2, 2016 at 1:11 PM, Kuntal Ghosh > wrote: >> Rest of the suggestions are well-taken. I'll update the patch accordingly. > I've updated the last submitted patch(v10) with

Re: [HACKERS] WAL consistency check facility

2016-11-02 Thread Kuntal Ghosh
On Wed, Nov 2, 2016 at 1:11 PM, Kuntal Ghosh wrote: > Rest of the suggestions are well-taken. I'll update the patch accordingly. I've updated the last submitted patch(v10) with the following changes: - added a block level flag BKPIMAGE_APPLY to distinguish backup image

Re: [HACKERS] WAL consistency check facility

2016-11-02 Thread Kuntal Ghosh
On Wed, Nov 2, 2016 at 1:34 PM, Michael Paquier wrote: > Hm... Right. That was broken. And actually, while the record-level > flag is useful so as you don't need to rely on checking > wal_consistency when doing WAL redo, the block-level flag is useful to > make a

Re: [HACKERS] WAL consistency check facility

2016-11-02 Thread Michael Paquier
On Wed, Nov 2, 2016 at 4:41 PM, Kuntal Ghosh wrote: > On Wed, Nov 2, 2016 at 10:23 AM, Michael Paquier > wrote: >> On Tue, Nov 1, 2016 at 10:31 PM, Robert Haas wrote: >>> IMHO, your rewrite of this patch was a bit

Re: [HACKERS] WAL consistency check facility

2016-11-02 Thread Kuntal Ghosh
On Wed, Nov 2, 2016 at 10:23 AM, Michael Paquier wrote: > On Tue, Nov 1, 2016 at 10:31 PM, Robert Haas wrote: >> IMHO, your rewrite of this patch was a bit heavy-handed. > > OK... Sorry for that. > >> I haven't >> scrutinized the code here so

Re: [HACKERS] WAL consistency check facility

2016-11-01 Thread Michael Paquier
On Tue, Nov 1, 2016 at 10:31 PM, Robert Haas 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

Re: [HACKERS] WAL consistency check facility

2016-11-01 Thread Peter Geoghegan
On Mon, Oct 31, 2016 at 5:31 AM, Robert Haas wrote: > On Fri, Oct 28, 2016 at 2:05 AM, Michael Paquier > wrote: >> And here we go. Here is a review as well as a large brush-up for this >> patch. A couple of things: >> - wal_consistency is using a

Re: [HACKERS] WAL consistency check facility

2016-11-01 Thread Robert Haas
On Mon, Oct 31, 2016 at 5:51 PM, Michael Paquier wrote: > Hehe, I was expecting you to jump on those lines. While looking at the > patch I have simplified it first to focus on the core engine of the > thing. Adding back this code sounds fine to me as there is a wall of

Re: [HACKERS] WAL consistency check facility

2016-10-31 Thread Michael Paquier
On Mon, Oct 31, 2016 at 9:31 PM, Robert Haas wrote: > On Fri, Oct 28, 2016 at 2:05 AM, Michael Paquier >> - Instead of palloc'ing the old and new pages to compare, it would be >> more performant to keep around two static buffers worth of BLCKSZ and >> just use that. This

Re: [HACKERS] WAL consistency check facility

2016-10-31 Thread Michael Paquier
On Mon, Oct 31, 2016 at 9:31 PM, Robert Haas wrote: > On Fri, Oct 28, 2016 at 2:05 AM, Michael Paquier > wrote: >> And here we go. Here is a review as well as a large brush-up for this >> patch. A couple of things: >> - wal_consistency is using a

Re: [HACKERS] WAL consistency check facility

2016-10-31 Thread Kuntal Ghosh
On Fri, Oct 28, 2016 at 11:35 AM, Michael Paquier wrote: > And here we go. Here is a review as well as a large brush-up for this > patch. A couple of things: Thanks for reviewing the patch in detail. > - Speaking of which using BKPIMAGE_IS_REQUIRED_FOR_REDO stored in

Re: [HACKERS] WAL consistency check facility

2016-10-31 Thread Robert Haas
On Fri, Oct 28, 2016 at 2:05 AM, Michael Paquier wrote: > And here we go. Here is a review as well as a large brush-up for this > patch. A couple of things: > - wal_consistency is using a list of RMGRs, at the cost of being > PGC_POSTMASTER. I'd suggest making it

Re: [HACKERS] WAL consistency check facility

2016-10-28 Thread Michael Paquier
On Thu, Oct 27, 2016 at 5:08 PM, Michael Paquier wrote: > On Thu, Sep 29, 2016 at 12:49 PM, Michael Paquier > wrote: >> Seeing nothing happening, I have moved the patch to next CF as there >> is a new version, but no reviews for it. > > Just

Re: [HACKERS] WAL consistency check facility

2016-10-27 Thread Michael Paquier
On Thu, Sep 29, 2016 at 12:49 PM, Michael Paquier wrote: > Seeing nothing happening, I have moved the patch to next CF as there > is a new version, but no reviews for it. Just a note for anybody potentially looking at this patch. I am currently looking at it in depth,

Re: [HACKERS] WAL consistency check facility

2016-09-28 Thread Michael Paquier
On Fri, Sep 16, 2016 at 10:36 PM, Michael Paquier wrote: > On Fri, Sep 16, 2016 at 10:30 PM, Robert Haas wrote: >> I don't think you have the right to tell Kuntal that he has to move >> the patch to the next CommitFest because there are

Re: [HACKERS] WAL consistency check facility

2016-09-16 Thread Michael Paquier
On Fri, Sep 16, 2016 at 10:30 PM, Robert Haas wrote: > I don't think you have the right to tell Kuntal that he has to move > the patch to the next CommitFest because there are unspecified things > about the current version you don't like. If you don't have time to > review

Re: [HACKERS] WAL consistency check facility

2016-09-16 Thread Robert Haas
On Thu, Sep 15, 2016 at 9:23 PM, Michael Paquier wrote: > On Thu, Sep 15, 2016 at 7:30 PM, Kuntal Ghosh > wrote: >> Thoughts? > > There are still a couple of things that this patch makes me unhappy, > particularly the handling of the GUC

Re: [HACKERS] WAL consistency check facility

2016-09-15 Thread Michael Paquier
On Thu, Sep 15, 2016 at 7:30 PM, Kuntal Ghosh wrote: > Thoughts? There are still a couple of things that this patch makes me unhappy, particularly the handling of the GUC with the xlogreader flags. I am not sure if I'll be able to look at that again within the next

Re: [HACKERS] WAL consistency check facility

2016-09-15 Thread Kuntal Ghosh
Hello, I've added the updated the patch with the necessary documentation and comments. I've referenced Robert's reply in this thread and Simon's reply in Production block comparison facility thread to write the documentation. This feature is used to check the consistency of WAL records, i.e,

Re: [HACKERS] WAL consistency check facility

2016-09-14 Thread Robert Haas
On Tue, Sep 13, 2016 at 9:04 PM, Michael Paquier wrote: > It seems to me that you need to think about the way to document things > properly first, with for example: > - Have a first documentation patch that explains what is a resource > manager for WAL, and what are the

Re: [HACKERS] WAL consistency check facility

2016-09-14 Thread Kuntal Ghosh
On Thu, Sep 8, 2016 at 1:20 PM, Michael Paquier wrote: > >> 2. For BRIN/UPDATE+INIT, block numbers (in rm_tid[0]) are different in >> REVMAP page. This happens only for two cases. I'm not sure what the >> reason can be. > > Hm? This smells like a block reference bug.

Re: [HACKERS] WAL consistency check facility

2016-09-14 Thread Kuntal Ghosh
On Wed, Sep 14, 2016 at 11:31 AM, Michael Paquier wrote: > On Wed, Sep 14, 2016 at 2:56 PM, Kuntal Ghosh > wrote: >> Master >> --- >> - If wal_consistency check is enabled or needs_backup is set in >> XLogRecordAssemble(), we do

Re: [HACKERS] WAL consistency check facility

2016-09-14 Thread Michael Paquier
On Wed, Sep 14, 2016 at 2:56 PM, Kuntal Ghosh wrote: > Master > --- > - If wal_consistency check is enabled or needs_backup is set in > XLogRecordAssemble(), we do a fpw. > - If a fpw is to be done, then fork_flags is set with BKPBLOCK_HAS_IMAGE, > which in

Re: [HACKERS] WAL consistency check facility

2016-09-13 Thread Kuntal Ghosh
On Wed, Sep 14, 2016 at 6:34 AM, Michael Paquier wrote: >>>2) Regarding page comparison: >>>We could just use memcpy() here. compareImages was useful to get a >>>clear image of what the inconsistencies were, but you don't do that >>>anymore. >> memcmp(), right? > >Yep

Re: [HACKERS] WAL consistency check facility

2016-09-13 Thread Michael Paquier
On Tue, Sep 13, 2016 at 6:07 PM, Kuntal Ghosh wrote: >> For now, I've kept this as a WARNING message to detect all inconsistencies >> at once. Once, the patch is finalized, I'll modify it as an ERROR message. > > Or say FATAL. This way the server is taken down. What

Re: [HACKERS] WAL consistency check facility

2016-09-13 Thread Kuntal Ghosh
- If WAL consistency check is enabled for a rmgrID, we always include the backup image in the WAL record. >>> >>>What happens if wal_consistency has different settings on a standby >>>and its master? If for example it is set to 'all' on the standby, and >>>'none' on the master, or

Re: [HACKERS] WAL consistency check facility

2016-09-13 Thread Michael Paquier
On Sun, Sep 11, 2016 at 12:03 AM, Robert Haas wrote: > It seems entirely unnecessary for the master and the standby to agree > here. I think what we need is two GUCs. One of them, which affects > only the master, controls whether the validation information is > including

Re: [HACKERS] WAL consistency check facility

2016-09-13 Thread Michael Paquier
On Mon, Sep 12, 2016 at 5:06 AM, Kuntal Ghosh wrote: >>+ void(*rm_checkConsistency) (XLogReaderState *record); >>All your _checkConsistency functions share the same pattern, in short >>they all use a for loop for each block, call each time

Re: [HACKERS] WAL consistency check facility

2016-09-11 Thread Kuntal Ghosh
Hello, Based on the previous discussions, I've modified the existing patch. >+ void(*rm_checkConsistency) (XLogReaderState *record); >All your _checkConsistency functions share the same pattern, in short >they all use a for loop for each block, call each time >XLogReadBufferExtended,

Re: [HACKERS] WAL consistency check facility

2016-09-10 Thread Amit Kapila
On Sat, Sep 10, 2016 at 8:33 PM, Robert Haas wrote: > On Sat, Sep 10, 2016 at 3:19 AM, Michael Paquier > wrote: >> On Fri, Sep 9, 2016 at 4:01 PM, Kuntal Ghosh >> wrote: > - If WAL consistency check is enabled

Re: [HACKERS] WAL consistency check facility

2016-09-10 Thread Amit Kapila
On Sat, Sep 10, 2016 at 12:49 PM, Michael Paquier wrote: > On Fri, Sep 9, 2016 at 4:01 PM, Kuntal Ghosh > wrote: > - In recovery tests (src/test/recovery/t), I've added wal_consistency parameter in the existing scripts. This

Re: [HACKERS] WAL consistency check facility

2016-09-10 Thread Robert Haas
On Sat, Sep 10, 2016 at 3:19 AM, Michael Paquier wrote: > On Fri, Sep 9, 2016 at 4:01 PM, Kuntal Ghosh > wrote: - If WAL consistency check is enabled for a rmgrID, we always include the backup image in the WAL record. >>> >>> What

Re: [HACKERS] WAL consistency check facility

2016-09-10 Thread Michael Paquier
On Fri, Sep 9, 2016 at 4:01 PM, Kuntal Ghosh wrote: >>> - If WAL consistency check is enabled for a rmgrID, we always include >>> the backup image in the WAL record. >> >> What happens if wal_consistency has different settings on a standby >> and its master? If for

Re: [HACKERS] WAL consistency check facility

2016-09-09 Thread Kuntal Ghosh
Hello Michael, Thanks for your detailed review. >> - If WAL consistency check is enabled for a rmgrID, we always include >> the backup image in the WAL record. > > What happens if wal_consistency has different settings on a standby > and its master? If for example it is set to 'all' on the

Re: [HACKERS] WAL consistency check facility

2016-09-08 Thread Michael Paquier
On Wed, Sep 7, 2016 at 7:22 PM, Kuntal Ghosh wrote: > Hello, Could you avoid top-posting please? More reference here: http://www.idallen.com/topposting.html > - If WAL consistency check is enabled for a rmgrID, we always include > the backup image in the WAL record.

Re: [HACKERS] WAL consistency check facility

2016-09-07 Thread Amit Kapila
On Wed, Sep 7, 2016 at 3:52 PM, Kuntal Ghosh wrote: > > I got two types of inconsistencies as following: > > 1. For Btree/UNLINK_PAGE_META, btpo_flags are different. In backup > page, BTP_DELETED and BTP_LEAF both the flags are set, whereas after > redo, only

Re: [HACKERS] WAL consistency check facility

2016-09-07 Thread Kuntal Ghosh
On Wed, Sep 7, 2016 at 3:52 PM, Kuntal Ghosh wrote: > Hello, > > As per the earlier discussions, I've attached the updated patch for > WAL consistency check feature. This is how the patch works: > The earlier patch (wal_consistency_v6.patch) was based on the commit id

Re: [HACKERS] WAL consistency check facility

2016-09-07 Thread Kuntal Ghosh
Hello, As per the earlier discussions, I've attached the updated patch for WAL consistency check feature. This is how the patch works: - If WAL consistency check is enabled for a rmgrID, we always include the backup image in the WAL record. - I've extended the RmgrTable with a new function

Re: [HACKERS] WAL consistency check facility

2016-09-01 Thread Peter Geoghegan
On Thu, Sep 1, 2016 at 9:23 AM, Robert Haas wrote: > Indeed, it had occurred to me that we might not even want to compile > this code into the server unless WAL_DEBUG is defined; after all, how > does it help a regular user to detect that the server has a bug? Bug > or no

Re: [HACKERS] WAL consistency check facility

2016-09-01 Thread Simon Riggs
On 1 September 2016 at 17:23, Robert Haas wrote: > The primary audience of this feature is PostgreSQL developers I have spoken to users who are waiting for this feature to run in production, which is why I suggested it. Some people care more about correctness than they

Re: [HACKERS] WAL consistency check facility

2016-09-01 Thread Robert Haas
On Thu, Sep 1, 2016 at 4:12 PM, Simon Riggs wrote: > So in my current understanding, a hinted change has by definition no > WAL record, so we just ship a FPW. Hmm. An FPW would have to be contained in a WAL record, so it can't be right to say that we ship an FPW for lack

Re: [HACKERS] WAL consistency check facility

2016-09-01 Thread Simon Riggs
On 1 September 2016 at 11:16, Robert Haas wrote: > On Wed, Aug 31, 2016 at 7:02 PM, Simon Riggs wrote: >> I'd prefer a solution that was not dependent upon RmgrID at all. >> >> If there are various special cases that we need to cater for, ISTM >>

Re: [HACKERS] WAL consistency check facility

2016-09-01 Thread Robert Haas
On Wed, Aug 31, 2016 at 7:02 PM, Simon Riggs wrote: > I'd prefer a solution that was not dependent upon RmgrID at all. > > If there are various special cases that we need to cater for, ISTM > they would be flaws in the existing WAL implementation rather than > anything we

Re: [HACKERS] WAL consistency check facility

2016-08-31 Thread Amit Kapila
On Thu, Sep 1, 2016 at 9:43 AM, Peter Geoghegan wrote: > On Wed, Aug 31, 2016 at 8:26 PM, Amit Kapila wrote: >>> As far as I am understanding things, we are aiming at something that >>> could be used on production systems. >>> >> >> I don't think you can

Re: [HACKERS] WAL consistency check facility

2016-08-31 Thread Peter Geoghegan
On Wed, Aug 31, 2016 at 8:26 PM, Amit Kapila wrote: >> As far as I am understanding things, we are aiming at something that >> could be used on production systems. >> > > I don't think you can enable it by default in production systems. > Enabling it will lead to

Re: [HACKERS] WAL consistency check facility

2016-08-31 Thread Amit Kapila
On Thu, Sep 1, 2016 at 8:02 AM, Amit Kapila wrote: > On Wed, Aug 31, 2016 at 7:02 PM, Simon Riggs wrote: >> On 27 August 2016 at 12:09, Kuntal Ghosh wrote: >> > * wal_consistency_mask = 511 /* Enable consistency

Re: [HACKERS] WAL consistency check facility

2016-08-31 Thread Amit Kapila
On Thu, Sep 1, 2016 at 8:30 AM, Michael Paquier wrote: > On Thu, Sep 1, 2016 at 11:32 AM, Amit Kapila wrote: >> On Wed, Aug 31, 2016 at 7:02 PM, Simon Riggs wrote: >>> On 27 August 2016 at 12:09, Kuntal Ghosh

Re: [HACKERS] WAL consistency check facility

2016-08-31 Thread Michael Paquier
On Thu, Sep 1, 2016 at 11:32 AM, Amit Kapila wrote: > On Wed, Aug 31, 2016 at 7:02 PM, Simon Riggs wrote: >> On 27 August 2016 at 12:09, Kuntal Ghosh wrote: >> > * wal_consistency_mask = 511 /* Enable consistency

Re: [HACKERS] WAL consistency check facility

2016-08-31 Thread Amit Kapila
On Wed, Aug 31, 2016 at 7:02 PM, Simon Riggs wrote: > On 27 August 2016 at 12:09, Kuntal Ghosh wrote: > * wal_consistency_mask = 511 /* Enable consistency check mask bit*/ >>> >>> What does this mean? (No docs) >> >> I was using this

Re: [HACKERS] WAL consistency check facility

2016-08-31 Thread Michael Paquier
On Wed, Aug 31, 2016 at 10:32 PM, Simon Riggs wrote: > On 27 August 2016 at 12:09, Kuntal Ghosh wrote: > * wal_consistency_mask = 511 /* Enable consistency check mask bit*/ >>> >>> What does this mean? (No docs) >> >> I was using this

  1   2   >