Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-03-09 Thread Peter Geoghegan
On Thu, Mar 9, 2017 at 7:12 PM, Peter Geoghegan wrote: >> Hm - I think it's fair to export RecentGlobalXmin, given that we >> obviously use it across modules in core code. I see very little reason >> not to export it. > > Well, the assertion is completely useless as anything but documentation...

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-03-09 Thread Peter Geoghegan
On Thu, Mar 9, 2017 at 7:10 PM, Andres Freund wrote: >> verify_nbtree.obj : error LNK2001: unresolved external symbol >> RecentGlobalXmin >> [C:\buildfarm\buildenv\HEAD\pgsql.build\amcheck.vcxproj] >> >> Rather than marking RecentGlobalXmin as PGDLLIMPORT, I'd rather just >> remove the documenti

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-03-09 Thread Andres Freund
Hi, On 2017-03-09 19:04:46 -0800, Peter Geoghegan wrote: > On Thu, Mar 9, 2017 at 4:41 PM, Andres Freund wrote: > > I don't really expect buildfarm fallout, but .. > > Unfortunately, there is some on Windows: Thanks for monitoring. > verify_nbtree.obj : error LNK2001: unresolved external sy

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-03-09 Thread Peter Geoghegan
On Thu, Mar 9, 2017 at 4:41 PM, Andres Freund wrote: > And pushed with these. Thanks. > I don't really expect buildfarm fallout, but .. Unfortunately, there is some on Windows: verify_nbtree.obj : error LNK2001: unresolved external symbol RecentGlobalXmin [C:\buildfarm\buildenv\HEAD\pgsql.bu

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-03-09 Thread Andres Freund
On 2017-03-09 16:29:24 -0800, Peter Geoghegan wrote: > I am generally in favor of more inclusive Reviewed-By lists. I suggest > that we expand it to: > > "Reviewed-By: Andres Freund, Thomas Vondra, Thomas Munro, Anastasia > Lubennikova, Robert Haas, Amit Langote" And pushed with these. I don't r

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-03-09 Thread Peter Geoghegan
On Thu, Mar 9, 2017 at 3:52 PM, Andres Freund wrote: > > But I also think it's more important to get some initial verification > functionality in, than resolving this conflict. I do, also quite > strongly, think we'll be better served with having something like what > you're proposing than nothing

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-03-09 Thread Andres Freund
On 2017-03-06 20:40:59 -0800, Peter Geoghegan wrote: > On Mon, Mar 6, 2017 at 3:57 PM, Andres Freund wrote: > > I'm ok with not immediately doing so, but I think Peter's design isn't > > in line with achieving something like this. > > I would be okay with doing this if we had a grab-bag of expensi

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-03-06 Thread Peter Geoghegan
On Mon, Mar 6, 2017 at 3:57 PM, Andres Freund wrote: > I'm ok with not immediately doing so, but I think Peter's design isn't > in line with achieving something like this. I would be okay with doing this if we had a grab-bag of expensive checks, that all pretty much require some combination of Ex

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-03-06 Thread Andres Freund
On 2017-02-13 12:05:21 -0500, Robert Haas wrote: > On Fri, Feb 10, 2017 at 8:39 PM, Peter Geoghegan wrote: > > On Thu, Feb 9, 2017 at 2:32 PM, Andres Freund wrote: > >> Except that the proposed names aren't remotely like that... ;). > > > > Revision attached -- V5. We now REVOKE ALL on both funct

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-03-06 Thread Andres Freund
Hi, On 2017-02-13 11:52:53 -0500, Robert Haas wrote: > On Thu, Feb 9, 2017 at 5:32 PM, Andres Freund wrote: > >> > Again, some parts of the code doing something bad isn't a good argument > >> > for doing again. Releasing locks early is a bad pattern, because backend > >> > code isn't generally sa

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-13 Thread Robert Haas
On Fri, Feb 10, 2017 at 8:39 PM, Peter Geoghegan wrote: > On Thu, Feb 9, 2017 at 2:32 PM, Andres Freund wrote: >> Except that the proposed names aren't remotely like that... ;). > > Revision attached -- V5. We now REVOKE ALL on both functions, as > Robert suggested, instead of the previous approa

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-13 Thread Robert Haas
On Thu, Feb 9, 2017 at 5:32 PM, Andres Freund wrote: >> > Again, some parts of the code doing something bad isn't a good argument >> > for doing again. Releasing locks early is a bad pattern, because backend >> > code isn't generally safe against it, we have special code-paths for >> > catalog tab

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-13 Thread Robert Haas
On Thu, Feb 9, 2017 at 8:15 PM, Peter Geoghegan wrote: > BTW, aren't there cases where a PARALLEL SAFE function that needs to > acquire locks on some arbitrary relation not referenced in the query > can get locks on its own, which may only last as long as the parallel > worker's lifetime? This cou

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-11 Thread Amit Kapila
On Fri, Feb 10, 2017 at 6:45 AM, Peter Geoghegan wrote: > On Thu, Feb 9, 2017 at 2:47 PM, Peter Geoghegan wrote: >>> which isn't an issue here, but reinforces my point about the (badly >>> documented) assumption that we don't release locks on user relations >>> early. >> I think even if we don't

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-10 Thread Peter Geoghegan
On Thu, Feb 9, 2017 at 2:32 PM, Andres Freund wrote: > Except that the proposed names aren't remotely like that... ;). Revision attached -- V5. We now REVOKE ALL on both functions, as Robert suggested, instead of the previous approach of having a hard-coded superuser check with enforcement. > An

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-09 Thread Peter Geoghegan
On Thu, Feb 9, 2017 at 2:47 PM, Peter Geoghegan wrote: >> which isn't an issue here, but reinforces my point about the (badly >> documented) assumption that we don't release locks on user relations >> early. > > You are right about the substantive issue, I assume, but I have a hard > time agreeing

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-09 Thread Peter Geoghegan
On Thu, Feb 9, 2017 at 2:32 PM, Andres Freund wrote: > Except that the proposed names aren't remotely like that... ;). > > And I proposed documenting named parameters, and > check_btree(performing_check_requiring_exclusive_locks => true) is just > about as expressive. I cannot think of an example

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-09 Thread Andres Freund
On 2017-02-09 17:12:58 -0500, Robert Haas wrote: > On Thu, Feb 9, 2017 at 4:57 PM, Andres Freund wrote: > > Meh. I don't think that's a serious problem, nor is without precedent > > (say same toplevel DDL with differing lock levels depending on > > options). Nor do the function names confer that

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-09 Thread Robert Haas
On Thu, Feb 9, 2017 at 4:57 PM, Andres Freund wrote: > On 2017-02-09 13:29:42 -0800, Peter Geoghegan wrote: >> > I'd really like to have something like relation_check(), index_check() >> > which calls the correct functions for the relevan index types (and >> > possibly warns about !nbtree indexes

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-09 Thread Andres Freund
On 2017-02-09 13:29:42 -0800, Peter Geoghegan wrote: > > I'd really like to have something like relation_check(), index_check() > > which calls the correct functions for the relevan index types (and > > possibly warns about !nbtree indexes not being checked?). > > I feel that that's something for

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-09 Thread Peter Geoghegan
On Thu, Feb 9, 2017 at 12:05 AM, Andres Freund wrote: > reindex_index isn't necessarily comparable, because > RangeVarCallbackForReindexIndex() is, on first blush at least, careful > enough about the locking, and reindex_relation only gets the index list > after building the index: > /* >

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-09 Thread Andres Freund
On 2017-02-09 14:48:18 -0500, Robert Haas wrote: > On Thu, Feb 9, 2017 at 3:05 AM, Andres Freund wrote: > >> +#define CHECK_SUPERUSER() { \ > >> + if (!superuser()) \ > >> + ereport(ERROR, \ > >> + > >> (errcode(ERRCODE_INSUFFICI

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-09 Thread Robert Haas
On Thu, Feb 9, 2017 at 3:05 AM, Andres Freund wrote: >> +#define CHECK_SUPERUSER() { \ >> + if (!superuser()) \ >> + ereport(ERROR, \ >> + >> (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), \ >> + (

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-02-09 Thread Andres Freund
Hi, On 2016-11-22 10:56:07 -0800, Peter Geoghegan wrote: > Andres said: "Theoretically we should recheck that the oids still > match, theoretically the locking here allows the index->table mapping > to change". I don't know how to act on this feedback, since comparable > index + heap locking code

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-01-30 Thread Andres Freund
Hi Heikki, On 2016-11-22 10:56:07 -0800, Peter Geoghegan wrote: > I attach V4 of amcheck. Is there any chance you could look at the concurrency related parts of amcheck? I'd like to push this to be mergeable, but that area is a bit outside of my area of expertise... Andres -- Sent via pgsql-

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2017-01-30 Thread Michael Paquier
On Mon, Dec 5, 2016 at 2:15 PM, Haribabu Kommi wrote: > Moved to next CF with " needs review" status. Same, to CF 2017-03 this time. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-12-04 Thread Haribabu Kommi
On Wed, Nov 30, 2016 at 9:06 AM, Peter Geoghegan wrote: > On Wed, Nov 23, 2016 at 2:47 PM, Thomas Munro > wrote: > > Actually I meant that at T2 the btree is exactly same as it was at T1, > > but the order of the values has changed according to the comparator > > (for example, because a collatio

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-29 Thread Peter Geoghegan
On Wed, Nov 23, 2016 at 2:47 PM, Thomas Munro wrote: > Actually I meant that at T2 the btree is exactly same as it was at T1, > but the order of the values has changed according to the comparator > (for example, because a collation definition changed when you updated > your operating system), so t

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-23 Thread Thomas Munro
On Fri, Nov 18, 2016 at 12:17 PM, Peter Geoghegan wrote: > So, the Btree you show is supposed to be good? You've left it up to me > to draw T2, the corrupt version? If so, here is a drawing of T2 for > the sake of clarity: > > Btree =[a|b] >/ \ >

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-22 Thread Peter Geoghegan
On Sun, Nov 20, 2016 at 3:42 PM, Robert Haas wrote: > OK. If it's not reasonable to continue checking after an ERROR, then > I think ERROR is the way to go. If somebody really doesn't like that > lack of flexibility (in either direction), they can propose a change > later for separate considerat

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-20 Thread Robert Haas
On Sat, Nov 19, 2016 at 11:38 PM, Peter Geoghegan wrote: > On Sat, Nov 19, 2016 at 6:45 PM, Robert Haas wrote: >>> What do you think about new argument with default vs. GUC? I guess >>> that the GUC might be a lot less of a foot-gun. We might even give it >>> a suitably scary name, to indicate th

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-19 Thread Peter Geoghegan
On Sat, Nov 19, 2016 at 6:45 PM, Robert Haas wrote: >> What do you think about new argument with default vs. GUC? I guess >> that the GUC might be a lot less of a foot-gun. We might even give it >> a suitably scary name, to indicate that it will make the server PANIC. >> (I gather that you don't c

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-19 Thread Robert Haas
On Fri, Nov 18, 2016 at 6:51 PM, Peter Geoghegan wrote: > On Thu, Nov 17, 2016 at 12:04 PM, Peter Geoghegan wrote: >>> Hm, if we want that - and it doesn't seem like a bad idea - I think we >>> should be make it available without recompiling. >> >> I suppose, provided it doesn't let CORRUPTION el

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-18 Thread Peter Geoghegan
On Thu, Nov 17, 2016 at 12:04 PM, Peter Geoghegan wrote: >> Hm, if we want that - and it doesn't seem like a bad idea - I think we >> should be make it available without recompiling. > > I suppose, provided it doesn't let CORRUPTION elevel be < ERROR. That > might be broken if it was allowed. Wha

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-17 Thread Peter Geoghegan
On Wed, Nov 16, 2016 at 5:06 PM, Thomas Munro wrote: > + * Ideally, we'd compare every item in the index against every other > + * item in the index, and not trust opclass obedience of the transitive > + * law to bridge the gap between children and their grandparents (as > + * well as great-grandp

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-17 Thread Peter Geoghegan
On Wed, Nov 16, 2016 at 1:03 PM, Andres Freund wrote: > I think the patch could use a pgindent run. Okay. > I'd really want a function that runs all check on a table. Why not just use a custom SQL query? The docs have an example of one such query, that verifies all user indexes, but there could

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-16 Thread Thomas Munro
On Thu, Sep 8, 2016 at 6:44 AM, Peter Geoghegan wrote: > On Fri, Sep 2, 2016 at 11:19 AM, Kevin Grittner wrote: >> IMV the process is to post a patch to this list to certify that it >> is yours to contribute and free of IP encumbrances that would >> prevent us from using it. I will wait for that

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-16 Thread Andres Freund
Hi, I think the patch could use a pgindent run. On 2016-09-07 11:44:01 -0700, Peter Geoghegan wrote: > diff --git a/contrib/amcheck/amcheck--1.0.sql > b/contrib/amcheck/amcheck--1.0.sql > new file mode 100644 > index 000..ebbd6ac > --- /dev/null > +++ b/contrib/amcheck/amcheck--1.0.sql > @@

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-10-16 Thread Michael Paquier
On Mon, Oct 17, 2016 at 10:46 AM, Noah Misch wrote: > - Detect impossible conditions in the hint bits. A tuple should not have both > HEAP_XMAX_COMMITTED and HEAP_XMAX_INVALID. Every tuple bearing > HEAP_ONLY_TUPLE should bear HEAP_UPDATED. HEAP_HASVARWIDTH should be true > if and only if

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-10-16 Thread Noah Misch
On Fri, Oct 14, 2016 at 04:56:39PM -0700, Peter Geoghegan wrote: > On Mon, Feb 29, 2016 at 4:09 PM, Peter Geoghegan wrote: > > To recap, the extension adds some SQL-callable functions that verify > > certain invariant conditions hold within some particular B-Tree index. > > These are the condition

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-10-14 Thread Peter Geoghegan
On Mon, Feb 29, 2016 at 4:09 PM, Peter Geoghegan wrote: > To recap, the extension adds some SQL-callable functions that verify > certain invariant conditions hold within some particular B-Tree index. > These are the conditions that index scans rely on always being true. > The tool's scope may even

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-10-06 Thread Michael Paquier
On Fri, Oct 7, 2016 at 8:05 AM, Peter Geoghegan wrote: > Your customer > databases might feature far more use of Japanese collations, for > example, which might be an important factor. Not really :) -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make chang

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-10-06 Thread Peter Geoghegan
On Sun, Oct 2, 2016 at 6:48 PM, Michael Paquier wrote: > Okay, moved to next CF. I may look at it finally I got some use-cases > for it, similar to yours I guess.. Let me know how that goes. One thing I've definitely noticed is that the tool is good at finding corner-case bugs, so even if you ca

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-10-02 Thread Michael Paquier
On Thu, Sep 8, 2016 at 3:44 AM, Peter Geoghegan wrote: > On Fri, Sep 2, 2016 at 11:19 AM, Kevin Grittner wrote: >> IMV the process is to post a patch to this list to certify that it >> is yours to contribute and free of IP encumbrances that would >> prevent us from using it. I will wait for that

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-09-12 Thread Peter Geoghegan
On Wed, Sep 7, 2016 at 11:44 AM, Peter Geoghegan wrote: > I attach my V3. I should point out that I'm leaving to go on Vacation this weekend. I'll be away for a week, and will not be answering mail during that period. If anyone has any questions on this for me, it might be preferable to ask them

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-09-07 Thread Peter Geoghegan
On Fri, Sep 2, 2016 at 11:19 AM, Kevin Grittner wrote: > IMV the process is to post a patch to this list to certify that it > is yours to contribute and free of IP encumbrances that would > prevent us from using it. I will wait for that post. I attach my V3. There are only very minor code change

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-09-02 Thread Peter Geoghegan
On Fri, Sep 2, 2016 at 11:16 AM, Peter Geoghegan wrote: > There are only tiny differences, which in any case you can see in the > commit log on Github. There is no reason why code review needs to > block on this V3, IMV. Also, I don't think that we need to include V2's tests now that there is ind

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-09-02 Thread Kevin Grittner
On Fri, Sep 2, 2016 at 1:16 PM, Peter Geoghegan wrote: > On Fri, Sep 2, 2016 at 11:11 AM, Kevin Grittner wrote: >> I changed the CF entry to "Waiting on Author" pending that. (I was >> starting to review the patch for commit, so it's none to early to >> mention that the last posted version is no

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-09-02 Thread Peter Geoghegan
On Fri, Sep 2, 2016 at 11:11 AM, Kevin Grittner wrote: > I changed the CF entry to "Waiting on Author" pending that. (I was > starting to review the patch for commit, so it's none to early to > mention that the last posted version is not what I should be > looking at.) There are only tiny differ

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-09-02 Thread Kevin Grittner
On Fri, Sep 2, 2016 at 11:31 AM, Peter Geoghegan wrote: > On Thu, Aug 18, 2016 at 5:14 PM, Peter Geoghegan wrote: >> I'd certainly welcome that. There are Debian packages available from >> the Github version of amcheck, which is otherwise practically >> identical to the most recent version of the

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-09-02 Thread Peter Geoghegan
On Thu, Aug 18, 2016 at 5:14 PM, Peter Geoghegan wrote: > I'd certainly welcome that. There are Debian packages available from > the Github version of amcheck, which is otherwise practically > identical to the most recent version of the patch posted here: > > https://github.com/petergeoghegan/amch

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-08-18 Thread Peter Geoghegan
On Thu, Aug 18, 2016 at 5:35 PM, Michael Paquier wrote: > This would be packaged from source in my case, but that's no big deal > :) At least I can see that it is added in the next CF, and that's > marked as ready for committer for a couple of months now... If you consider how the code is written

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-08-18 Thread Michael Paquier
On Fri, Aug 19, 2016 at 9:14 AM, Peter Geoghegan wrote: > On Thu, Aug 18, 2016 at 5:06 PM, Michael Paquier > wrote: >> Cool. I have been honestly wondering about deploying this tool as well >> to allow some of the QE tests to perform live checks of btree indexes >> as we use a bunch of them. > >

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-08-18 Thread Peter Geoghegan
On Thu, Aug 18, 2016 at 5:06 PM, Michael Paquier wrote: > Cool. I have been honestly wondering about deploying this tool as well > to allow some of the QE tests to perform live checks of btree indexes > as we use a bunch of them. I'd certainly welcome that. There are Debian packages available fro

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-08-18 Thread Michael Paquier
On Fri, Aug 19, 2016 at 2:40 AM, Peter Geoghegan wrote: > Heroku began a selective roll-out of amcheck yesterday. amcheck > already found a bug in the PostGiS Geography B-Tree opclass: > [...] > I'll go report this to the PostGiS people. Cool. I have been honestly wondering about deploying this t

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-08-18 Thread Peter Geoghegan
On Sat, Mar 12, 2016 at 12:38 PM, Peter Geoghegan wrote: > Only insofar as it helps diagnose the underlying issue, when it is a > more subtle issue. Actually fixing the index is almost certainly a > REINDEX. Once you're into the messy business of diagnosing a > problematic opclass, you have to be

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-04-08 Thread Anastasia Lubennikova
29.03.2016 06:13, Peter Geoghegan: On Tue, Mar 22, 2016 at 10:57 AM, Peter Geoghegan wrote: That's right - I have a small number of feedback items to work through. I also determined myself that there could be a very low probability race condition when checking the key space across sibling pages

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-28 Thread Peter Geoghegan
On Tue, Mar 22, 2016 at 10:57 AM, Peter Geoghegan wrote: > That's right - I have a small number of feedback items to work > through. I also determined myself that there could be a very low > probability race condition when checking the key space across sibling > pages, and will work to address tha

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-22 Thread Peter Geoghegan
On Tue, Mar 22, 2016 at 10:57 AM, Peter Geoghegan wrote: > That's right - I have a small number of feedback items to work > through. I also determined myself that there could be a very low > probability race condition when checking the key space across sibling > pages, and will work to address tha

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-22 Thread Peter Geoghegan
On Tue, Mar 22, 2016 at 9:33 AM, David Steele wrote: > It looks like an updated patch is expected here, though it seems that > the only requests are for updates to comments. That's right - I have a small number of feedback items to work through. I also determined myself that there could be a very

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-22 Thread David Steele
On 3/15/16 3:42 AM, Peter Geoghegan wrote: > On Tue, Mar 15, 2016 at 12:31 AM, Amit Langote > wrote: >> Ah, I see the nuance. Thanks for the explanation. Maybe, >> bt_index_check() and bt_index_parent_child_check() / >> bt_index_check_parent_child(). IMHO, the latter more clearly highlights >>

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-15 Thread Amit Langote
On 2016/03/15 16:42, Peter Geoghegan wrote: > On Tue, Mar 15, 2016 at 12:31 AM, Amit Langote > wrote: >> By the way, one request (as a non-native speaker of English language, who >> ends up looking up quite a few words regularly) - >> >> Could we use "conform" or "correspond" instead of "comport"

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-15 Thread Peter Geoghegan
On Tue, Mar 15, 2016 at 12:31 AM, Amit Langote wrote: > Ah, I see the nuance. Thanks for the explanation. Maybe, > bt_index_check() and bt_index_parent_child_check() / > bt_index_check_parent_child(). IMHO, the latter more clearly highlights > the fact that parent/child relationships in the for

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-15 Thread Amit Langote
Hi Peter, On 2016/03/15 16:11, Peter Geoghegan wrote: > On Mon, Mar 14, 2016 at 11:48 PM, Amit Langote > wrote: >>> Dunno about that. It's defining characteristic is that it checks child >>> pages against their parent IMV. Things are not often defined in terms >>> of their locking requirements.

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-15 Thread Peter Geoghegan
On Mon, Mar 14, 2016 at 11:48 PM, Amit Langote wrote: >> Dunno about that. It's defining characteristic is that it checks child >> pages against their parent IMV. Things are not often defined in terms >> of their locking requirements. > > At the risk of sounding a bit verbose, do bt_check_level()

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-14 Thread Amit Langote
On 2016/03/12 6:31, Peter Geoghegan wrote: > On Thu, Mar 10, 2016 at 9:18 AM, Tomas Vondra > wrote: >> I've looked at this patch today, mostly to educate myself, so this >> probably should not count as a full review. Anyway, the patch seems in >> excellent shape - it'd be great if all patches (inc

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-14 Thread Peter Geoghegan
On Fri, Mar 11, 2016 at 8:19 AM, Anastasia Lubennikova wrote: > But I have some concerns about compatibility with my patches. > I've tried to call bt_index_check() over my "including" patch [1] and caught > a segfault. > > LOG: server process (PID 31794) was terminated by signal 11: Segmentation

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-14 Thread Peter Geoghegan
On Mon, Mar 14, 2016 at 10:39 AM, Robert Haas wrote: > I don't particularly like that interface. I also suggest that it > would be better to leave throttling to a future commit, and focus on > getting the basic feature in first. Works for me. I don't think throttling is an especially compelling

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-14 Thread Robert Haas
On Fri, Mar 11, 2016 at 7:45 PM, Peter Geoghegan wrote: > On Fri, Mar 11, 2016 at 4:30 PM, Jim Nasby wrote: >> Right, but you still have the option to enable them if you don't want to >> swamp your IO system. That's why CIC obeys it too. If I was running a >> consistency check on a production sys

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-13 Thread Jim Nasby
On 3/11/16 6:45 PM, Peter Geoghegan wrote: I'll add that if people like the interface you propose. (Overloading the VACUUM cost delay functions to cause a delay for amcheck functions, too). I thought that had already been overloaded by CIC, but I'm not finding reference to it... ANALYZE does u

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-13 Thread Peter Geoghegan
On Sat, Mar 12, 2016 at 7:46 PM, Matt Kelly wrote: > You can actually pretty easily produce a test case by setting up streaming > replication between servers running two different version of glibc. > > I actually wrote a tool that spins up a pair of VMs using vagrant and then > sets them up as str

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-12 Thread Matt Kelly
> > On Fri, 2016-03-11 at 17:24 +0100, Michael Paquier wrote: > > On Fri, Mar 11, 2016 at 5:19 PM, Anastasia Lubennikova wrote: > > > > > > BTW, if you know a good way to corrupt index (and do it > > > reproducible) I'd be > > > very glad to see it. > > You can use for example dd in non-truncate mo

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-12 Thread Peter Geoghegan
On Fri, Mar 11, 2016 at 6:33 PM, Tomas Vondra wrote: > Right, but isn't there a difference between the two functions in this > respect? Once you find corruption involving relationship between > multiple pages, then I agree it's complicated to do any reasoning about > what additional checks are saf

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Tomas Vondra
On Fri, 2016-03-11 at 16:40 -0800, Peter Geoghegan wrote: > On Fri, Mar 11, 2016 at 4:17 PM, Peter Geoghegan > wrote: > > > > If you want the tool to limp on when it finds an error, that can be > > done by changing the constant for the CORRUPTION macro in > > amcheck.c. > > But having that be dy

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Peter Geoghegan
On Fri, Mar 11, 2016 at 4:30 PM, Jim Nasby wrote: > Right, but you still have the option to enable them if you don't want to > swamp your IO system. That's why CIC obeys it too. If I was running a > consistency check on a production system I'd certainly want the option to > throttle it. Without th

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Peter Geoghegan
On Fri, Mar 11, 2016 at 4:17 PM, Peter Geoghegan wrote: > If you want the tool to limp on when it finds an error, that can be > done by changing the constant for the CORRUPTION macro in amcheck.c. > But having that be dynamically configurable is not really compatible > with the goal of having amch

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Jim Nasby
On 3/11/16 6:17 PM, Peter Geoghegan wrote: Not sure about the cost delay thing. Delays are disabled by default for manually issued VACUUM, so have doubts that that's useful. Right, but you still have the option to enable them if you don't want to swamp your IO system. That's why CIC obeys it t

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Peter Geoghegan
On Fri, Mar 11, 2016 at 3:50 PM, Jim Nasby wrote: > I also agree that the nmodule name isn't very clear. If this is meant to be > the start of a generic consistency checker, lets call it that. Otherwise, it > should be marked as being specific to btrees, because presumably we might > eventually wa

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Jim Nasby
On 3/11/16 3:31 PM, Peter Geoghegan wrote: Can we come up with names that more clearly identify the difference >between those two functions? I mean,_parent_ does not make it >particularly obvious that the second function acquires exclusive lock >and performs more thorough checks. Dunno about th

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Peter Geoghegan
On Fri, Mar 11, 2016 at 8:19 AM, Anastasia Lubennikova wrote: > I do hope that my patch will be accepted in 9.6, so this conflict looks > really bad. I hope so too. I'll have to look into this issue. > I think that error is caused by changes in pages layout. To save some space > nonkey attribute

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Peter Geoghegan
On Fri, Mar 11, 2016 at 1:31 PM, Peter Geoghegan wrote: > You could have a race, where > there was a concurrent page deletion of the left sibling of the child > page, then a concurrent insertion into the newly expanded keyspace of > the parent. Therefore, the downlink in the parent (which is the >

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Peter Geoghegan
On Thu, Mar 10, 2016 at 9:18 AM, Tomas Vondra wrote: > I've looked at this patch today, mostly to educate myself, so this > probably should not count as a full review. Anyway, the patch seems in > excellent shape - it'd be great if all patches (including those written > by me) had this level of co

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Peter Geoghegan
On Fri, Mar 11, 2016 at 1:09 PM, Peter Geoghegan wrote: > Or, you could add code like this to comparetup_index_btree(), to > simulate a broken opclass: > > diff --git a/src/backend/utils/sort/tuplesort.c > b/src/backend/utils/sort/tuplesort.c > index 67d86ed..23712ff 100644 > --- a/src/backend/uti

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Peter Geoghegan
On Fri, Mar 11, 2016 at 8:24 AM, Michael Paquier wrote: > You can use for example dd in non-truncate mode to corrupt on-disk > page data, say that for example: > dd if=/dev/random bs=8192 count=1 \ > seek=$BLOCK_ID of=base/$DBOID/$RELFILENODE \ > conv=notrunc Sure, but that would probably

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Tomas Vondra
Hi, On Fri, 2016-03-11 at 17:24 +0100, Michael Paquier wrote: > On Fri, Mar 11, 2016 at 5:19 PM, Anastasia Lubennikova wrote: > > > > BTW, if you know a good way to corrupt index (and do it > > reproducible) I'd be > > very glad to see it. > You can use for example dd in non-truncate mode to corr

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Michael Paquier
On Fri, Mar 11, 2016 at 5:19 PM, Anastasia Lubennikova wrote: > BTW, if you know a good way to corrupt index (and do it reproducible) I'd be > very glad to see it. You can use for example dd in non-truncate mode to corrupt on-disk page data, say that for example: dd if=/dev/random bs=8192 count=1

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-11 Thread Anastasia Lubennikova
01.03.2016 03:09, Peter Geoghegan: I was assigned an "action point" during the FOSDEM developer meeting: "Post new version of btree consistency checker patch". I attach a new WIP version of my consistency checker tool, amcheck. This patch is proposed for 9.6, as an extension in contrib -- maybe w

Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-03-10 Thread Tomas Vondra
Hi, I've looked at this patch today, mostly to educate myself, so this probably should not count as a full review. Anyway, the patch seems in excellent shape - it'd be great if all patches (including those written by me) had this level of comments/docs. On Mon, 2016-02-29 at 16:09 -0800, Peter Ge

[HACKERS] amcheck (B-Tree integrity checking tool)

2016-02-29 Thread Peter Geoghegan
I was assigned an "action point" during the FOSDEM developer meeting: "Post new version of btree consistency checker patch". I attach a new WIP version of my consistency checker tool, amcheck. This patch is proposed for 9.6, as an extension in contrib -- maybe we can still get it in. This is the fi