Horton

Re: Online checksums verification in the backend

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Michael Paquier <michael@paquier.xyz>
Cc: Julien Rouhaud <rjuju123@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Robert Haas <robertmhaas@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Masahiko Sawada <sawada.mshk@gmail.com>
Date: 2020-10-29T18:31:06Z
Lists: pgsql-hackers 
Hi,

On 2020-10-29 11:17:29 -0700, Andres Freund wrote:
> 		LWLockAcquire(BufferDescriptorGetIOLock(bufdesc), LW_SHARED);
> 		buf_state = LockBufHdr(bufdesc);
> 		UnlockBufHdr(bufdesc, buf_state);
> 
> 		/* If the page is dirty or invalid, skip it */
> 		if ((buf_state & BM_DIRTY) != 0 || (buf_state & BM_TAG_VALID) == 0)

This is weird as well. What is this supposed to do? Just locking and
unlocking a buffer header doesn't do squat? There's no guarantee that
the flags haven't changed by this point, so you could just as well not
acquire the buffer header lock.

Also, why are pages without a valid tag ignored? I can follow the
argument for skipping it in the DIRTY case, but that doesn't apply for
BM_TAG_VALID?

Greetings,

Andres Freund

Commits

  1. Revert pg_relation_check_pages()

  2. Fix incorrect placement of pfree() in pg_relation_check_pages()

  3. Add pg_relation_check_pages() to check on-disk pages of a relation

  4. Add CheckBuffer() to check on-disk pages without shared buffer loading

  5. Extend amcheck to check heap pages.