Date: Tue, 30 Jun 2020 14:44:44 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>, Peter Geoghegan <pg@bowt.ie>,
	Andres Freund <andres@anarazel.de>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: new heapcheck contrib module
Message-ID: <20200630184444.GA15974@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <5CC85F28-F210-47AC-8DA8-EC4E82AF3FD5@enterprisedb.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

I think there are two very large patches here.  One adds checking of
heapam tables to amcheck, and the other adds a binary that eases calling
amcheck from the command line.  I think these should be two separate
patches.

I don't know what to think of a module contrib/pg_amcheck.  I kinda lean
towards fitting it in src/bin/scripts rather than as a contrib module.
However, it seems a bit weird that it depends on a contrib module.
Maybe amcheck should not be a contrib module at all but rather a new
extension in src/extensions/ that is compiled and installed (in the
filesystem, not in databases) by default.

I strongly agree with hardening backend code so that all the crashes
that Mark has found can be repaired.  (We discussed this topic
before[1]: we'd repair all crashes when run with production code, not
all assertion crashes.)

[1] https://postgr.es/m/20200513221051.GA26592@alvherre.pgsql

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services