Re: Online checksums verification in the backend

2020-11-03 Thread Michael Paquier
On Tue, Nov 03, 2020 at 06:46:12PM +0900, Michael Paquier wrote: > Yep, that's clear. I'll deal with that tomorrow. That's more than a > simple rework. This part is now done as of e152506a. -- Michael signature.asc Description: PGP signature

Re: Online checksums verification in the backend

2020-11-03 Thread Robert Haas
On Mon, Nov 2, 2020 at 2:35 PM Andres Freund wrote: > Wouldn't this be better served by having a ReadBufferExtended() flag, > preventing erroring out and zeroing the buffer? I'm not sure that > handling both the case where the buffer contents need to be valid and > the one where it doesn't will

Re: Online checksums verification in the backend

2020-11-03 Thread Michael Paquier
On Mon, Nov 02, 2020 at 11:34:57AM -0800, Andres Freund wrote: > On 2020-11-02 12:35:30 -0500, Robert Haas wrote: >> On Thu, Oct 29, 2020 at 2:17 PM Andres Freund wrote: >>> I think this needs to be quickly reworked or reverted. > > I think it's fairly clear by now that revert is appropriate for

Re: Online checksums verification in the backend

2020-11-03 Thread Michael Paquier
On Tue, Nov 03, 2020 at 09:31:20AM +0100, Magnus Hagander wrote: > On Mon, Nov 2, 2020 at 8:35 PM Andres Freund wrote: >> On 2020-11-02 12:35:30 -0500, Robert Haas wrote: >>> It feels really confusing to me that the user-exposed function here is >>> called pg_relation_check_pages(). How is the

Re: Online checksums verification in the backend

2020-11-03 Thread Magnus Hagander
On Mon, Nov 2, 2020 at 8:35 PM Andres Freund wrote: > > Hi, > > On 2020-11-02 12:35:30 -0500, Robert Haas wrote: > > It feels really confusing to me that the user-exposed function here is > > called pg_relation_check_pages(). How is the user supposed to > > understand the difference between what

Re: Online checksums verification in the backend

2020-11-02 Thread Andres Freund
Hi, On 2020-11-02 12:35:30 -0500, Robert Haas wrote: > On Thu, Oct 29, 2020 at 2:17 PM Andres Freund wrote: > > I think this needs to be quickly reworked or reverted. I think it's fairly clear by now that revert is appropriate for now. > I don't like this patch, either. In addition to what

Re: Online checksums verification in the backend

2020-11-02 Thread Robert Haas
On Thu, Oct 29, 2020 at 2:17 PM Andres Freund wrote: > I think this needs to be quickly reworked or reverted. I don't like this patch, either. In addition to what Andres mentioned, CheckBuffer() is a completely-special case mechanism which can't be reused by anything else. In particular, the

Re: Online checksums verification in the backend

2020-11-01 Thread Michael Paquier
On Sun, Nov 01, 2020 at 05:50:06PM -0800, Andres Freund wrote: > On 2020-11-02 10:45:00 +0900, Michael Paquier wrote: > > On 2020-11-02 10:05:25 +0900, Michael Paquier wrote: > > > > There is no place doing that on HEAD. > > > > > > Err? > > > How is this not doing IO while holding a buffer

Re: Online checksums verification in the backend

2020-11-01 Thread Andres Freund
Hi On 2020-11-02 10:45:00 +0900, Michael Paquier wrote: > On Sun, Nov 01, 2020 at 05:39:40PM -0800, Andres Freund wrote: > > I'm a bit limited writing - one handed for a while following an injury > > on Friday... > > Oops. Take care. Thanks! > > On 2020-11-02 10:05:25 +0900, Michael Paquier

Re: Online checksums verification in the backend

2020-11-01 Thread Michael Paquier
On Sun, Nov 01, 2020 at 05:39:40PM -0800, Andres Freund wrote: > I'm a bit limited writing - one handed for a while following an injury > on Friday... Oops. Take care. > On 2020-11-02 10:05:25 +0900, Michael Paquier wrote: > > There is no place doing that on HEAD. > > Err? > > /* see if

Re: Online checksums verification in the backend

2020-11-01 Thread Andres Freund
Hi, I'm a bit limited writing - one handed for a while following an injury on Friday... On 2020-11-02 10:05:25 +0900, Michael Paquier wrote: > On Thu, Oct 29, 2020 at 10:08:52PM -0700, Andres Freund wrote: > > I think its pretty much *never* OK to do IO while holding a buffer > > mapping lock.

Re: Online checksums verification in the backend

2020-11-01 Thread Michael Paquier
On Thu, Oct 29, 2020 at 10:08:52PM -0700, Andres Freund wrote: > I think its pretty much *never* OK to do IO while holding a buffer > mapping lock. You're locking a significant fraction of shared buffers > over IO. That's just not OK. Don't think there's any place doing so > currently either.

Re: Online checksums verification in the backend

2020-10-29 Thread Andres Freund
On 2020-10-30 11:58:13 +0800, Julien Rouhaud wrote: > So I'm assuming that the previous optimization to avoid almost every > time doing an IO while holding a buffer mapping lock isn't an option? > In that case, I don't see any other option than reverting the patch > and discussing a new approach.

Re: Online checksums verification in the backend

2020-10-29 Thread Julien Rouhaud
On Fri, Oct 30, 2020 at 10:58 AM Andres Freund wrote: > > Hi, > > On 2020-10-30 10:01:08 +0800, Julien Rouhaud wrote: > > On Fri, Oct 30, 2020 at 2:17 AM Andres Freund wrote: > > > The code does IO while holding the buffer mapping lock. That seems > > > *entirely* unacceptable to me. That

Re: Online checksums verification in the backend

2020-10-29 Thread Andres Freund
Hi, On 2020-10-30 10:01:08 +0800, Julien Rouhaud wrote: > On Fri, Oct 30, 2020 at 2:17 AM Andres Freund wrote: > > The code does IO while holding the buffer mapping lock. That seems > > *entirely* unacceptable to me. That basically locks 1/128 of shared > > buffers against concurrent mapping

Re: Online checksums verification in the backend

2020-10-29 Thread Julien Rouhaud
Hi, On Fri, Oct 30, 2020 at 2:17 AM Andres Freund wrote: > The code does IO while holding the buffer mapping lock. That seems > *entirely* unacceptable to me. That basically locks 1/128 of shared > buffers against concurrent mapping changes, while reading data that is > likely not to be on disk?

Re: Online checksums verification in the backend

2020-10-29 Thread Andres Freund
Hi, On 2020-10-29 11:17:29 -0700, Andres Freund wrote: > The code does IO while holding the buffer mapping lock. That seems > *entirely* unacceptable to me. That basically locks 1/128 of shared > buffers against concurrent mapping changes, while reading data that is > likely not to be on disk?

Re: Online checksums verification in the backend

2020-10-29 Thread Andres Freund
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 */

Re: Online checksums verification in the backend

2020-10-29 Thread Andres Freund
Hi, On 2020-10-28 14:08:52 +0900, Michael Paquier wrote: > Thanks for confirming. I have gone through the whole set today, > splitted the thing into two commits and applied them. We had > buildfarm member florican complain about a mistake in one of the > GetDatum() calls that I took care of

Re: Online checksums verification in the backend

2020-10-28 Thread Julien Rouhaud
Hello, On Thu, Oct 29, 2020 at 7:52 AM Shinoda, Noriyoshi (PN Japan A Delivery) wrote: > > Hi, > > I have tested this great feature in the latest commit environment on Red Hat > Enterprise Linux 7.8. I modified a few blocks in a relation file to raise a > checksum error. When I executed the

RE: Online checksums verification in the backend

2020-10-28 Thread Shinoda, Noriyoshi (PN Japan A Delivery)
is the operation log. Regards, Noriyoshi Shinoda -Original Message- From: Michael Paquier [mailto:mich...@paquier.xyz] Sent: Wednesday, October 28, 2020 2:09 PM To: Julien Rouhaud Cc: Justin Pryzby ; Masahiko Sawada ; Robert Haas ; PostgreSQL Hackers ; Masahiko Sawada Subject: Re: Online checksums

Re: Online checksums verification in the backend

2020-10-27 Thread Michael Paquier
On Tue, Oct 27, 2020 at 07:47:19PM +0800, Julien Rouhaud wrote: > I think it's also worth noting that the IOLock is now acquired just > before getting the buffer state, and released after the read (or after > finding that the buffer is dirty). This is consistent with how it's > done elsewhere, so

Re: Online checksums verification in the backend

2020-10-27 Thread Julien Rouhaud
On Tue, Oct 27, 2020 at 3:07 PM Michael Paquier wrote: > > On Fri, Oct 23, 2020 at 06:06:30PM +0900, Michael Paquier wrote: > > On Fri, Oct 23, 2020 at 04:31:56PM +0800, Julien Rouhaud wrote: > >> Mmm, is it really an improvement to report warnings during this > >> function execution? Note also

Re: Online checksums verification in the backend

2020-10-27 Thread Michael Paquier
On Fri, Oct 23, 2020 at 06:06:30PM +0900, Michael Paquier wrote: > On Fri, Oct 23, 2020 at 04:31:56PM +0800, Julien Rouhaud wrote: >> Mmm, is it really an improvement to report warnings during this >> function execution? Note also that PageIsVerified as-is won't report >> a warning if a page is

Re: Online checksums verification in the backend

2020-10-23 Thread Michael Paquier
On Fri, Oct 23, 2020 at 04:31:56PM +0800, Julien Rouhaud wrote: > I agree. However I'm assuming that this refactor is relying on the > not yet committed patch (in the nearby thread dealing with base backup > checksums check) to also refactor PageIsVerified? As all the logic > you removed was

Re: Online checksums verification in the backend

2020-10-23 Thread Julien Rouhaud
On Fri, Oct 23, 2020 at 3:28 PM Michael Paquier wrote: > > On Mon, Oct 19, 2020 at 04:52:48PM +0900, Michael Paquier wrote: > > No issues with reporting the block number and the fork type in the SRF > > at least of course as this is helpful to detect the position of the > > broken blocks. For

Re: Online checksums verification in the backend

2020-10-23 Thread Michael Paquier
On Mon, Oct 19, 2020 at 04:52:48PM +0900, Michael Paquier wrote: > No issues with reporting the block number and the fork type in the SRF > at least of course as this is helpful to detect the position of the > broken blocks. For the checksum found in the header and the one > calculated (named

Re: Online checksums verification in the backend

2020-10-19 Thread Michael Paquier
On Mon, Oct 19, 2020 at 11:16:38AM +0800, Julien Rouhaud wrote: > On Mon, Oct 19, 2020 at 10:39 AM Michael Paquier wrote: >> Somewhat related to the first point, NoComputedChecksum exists >> because, as the current patch is shaped, we need to report an existing >> checksum to the user even for

Re: Online checksums verification in the backend

2020-10-18 Thread Julien Rouhaud
On Mon, Oct 19, 2020 at 10:39 AM Michael Paquier wrote: > > On Fri, Oct 16, 2020 at 09:22:02AM +0800, Julien Rouhaud wrote: > > And Michael just told me that I also missed adding one of the C files > > while splitting the patch into two. > > + if (PageIsNew(page)) > + { > + /* > +

Re: Online checksums verification in the backend

2020-10-18 Thread Michael Paquier
On Fri, Oct 16, 2020 at 09:22:02AM +0800, Julien Rouhaud wrote: > And Michael just told me that I also missed adding one of the C files > while splitting the patch into two. + if (PageIsNew(page)) + { + /* +* Check if the page is really new or if there's corruption that +

Re: Online checksums verification in the backend

2020-10-15 Thread Julien Rouhaud
On Fri, Oct 16, 2020 at 8:59 AM Julien Rouhaud wrote: > > On Thu, Oct 15, 2020 at 3:59 PM Julien Rouhaud wrote: > > > > On Thu, Oct 15, 2020 at 1:37 PM Julien Rouhaud wrote: > > > > > > On Thu, Oct 1, 2020 at 1:07 PM Michael Paquier > > > wrote: > > > > > > > > On Fri, Sep 25, 2020 at

Re: Online checksums verification in the backend

2020-10-15 Thread Julien Rouhaud
On Thu, Oct 15, 2020 at 3:59 PM Julien Rouhaud wrote: > > On Thu, Oct 15, 2020 at 1:37 PM Julien Rouhaud wrote: > > > > On Thu, Oct 1, 2020 at 1:07 PM Michael Paquier wrote: > > > > > > On Fri, Sep 25, 2020 at 06:11:47PM +0800, Julien Rouhaud wrote: > > > > Thanks a lot for the tests! I'm not

Re: Online checksums verification in the backend

2020-10-15 Thread Julien Rouhaud
On Thu, Oct 15, 2020 at 1:37 PM Julien Rouhaud wrote: > > On Thu, Oct 1, 2020 at 1:07 PM Michael Paquier wrote: > > > > On Fri, Sep 25, 2020 at 06:11:47PM +0800, Julien Rouhaud wrote: > > > Thanks a lot for the tests! I'm not surprised that forcing the lock > > > will slow down the

Re: Online checksums verification in the backend

2020-10-14 Thread Julien Rouhaud
On Thu, Oct 1, 2020 at 1:07 PM Michael Paquier wrote: > > On Fri, Sep 25, 2020 at 06:11:47PM +0800, Julien Rouhaud wrote: > > Thanks a lot for the tests! I'm not surprised that forcing the lock > > will slow down the pg_check_relation() execution, but I'm a bit > > surprised that holding the

Re: Online checksums verification in the backend

2020-09-30 Thread Michael Paquier
On Fri, Sep 25, 2020 at 06:11:47PM +0800, Julien Rouhaud wrote: > Thanks a lot for the tests! I'm not surprised that forcing the lock > will slow down the pg_check_relation() execution, but I'm a bit > surprised that holding the buffer mapping lock longer in a workload > that has a lot of

Re: Online checksums verification in the backend

2020-09-25 Thread Julien Rouhaud
On Wed, Sep 16, 2020 at 11:46 AM Michael Paquier wrote: > > On Fri, Sep 11, 2020 at 09:49:16AM +0200, Julien Rouhaud wrote: > > Thanks! > > I got some numbers out of my pocket, using the following base > configuration: > [...] > > Within parenthesis is the amount of time taken by

Re: Online checksums verification in the backend

2020-09-15 Thread Michael Paquier
On Fri, Sep 11, 2020 at 09:49:16AM +0200, Julien Rouhaud wrote: > Thanks! I got some numbers out of my pocket, using the following base configuration: wal_level = minimal fsync = off shared_buffers = '300MB' # also tested with 30MB and 60MB checksum_cost_delay = 0 # default in patch And for the

Re: Online checksums verification in the backend

2020-09-11 Thread Julien Rouhaud
On Fri, Sep 11, 2020 at 9:34 AM Michael Paquier wrote: > > On Thu, Sep 10, 2020 at 08:06:10PM +0200, Julien Rouhaud wrote: > > The TPS is obviously overall extremely bad, but I can see that the submitted > > version added an overhead of ~3.9% (average of 5 runs), while the version > > without the

Re: Online checksums verification in the backend

2020-09-11 Thread Michael Paquier
On Thu, Sep 10, 2020 at 08:06:10PM +0200, Julien Rouhaud wrote: > The TPS is obviously overall extremely bad, but I can see that the submitted > version added an overhead of ~3.9% (average of 5 runs), while the version > without the optimisation added an overhead of ~6.57%. Okay, so that stands

Re: Online checksums verification in the backend

2020-09-10 Thread Julien Rouhaud
On Thu, Sep 10, 2020 at 09:47:23AM +0200, Julien Rouhaud wrote: > On Wed, Sep 09, 2020 at 03:41:30PM +0200, Julien Rouhaud wrote: > > On Wed, Sep 09, 2020 at 11:25:29AM +0200, Julien Rouhaud wrote: > > > > > > I'll do some becnhmarking and see if I can get some figures, but it'll > > > probably

Re: Online checksums verification in the backend

2020-09-10 Thread Julien Rouhaud
On Wed, Sep 09, 2020 at 03:41:30PM +0200, Julien Rouhaud wrote: > On Wed, Sep 09, 2020 at 11:25:29AM +0200, Julien Rouhaud wrote: > > > > I'll do some becnhmarking and see if I can get some figures, but it'll > > probably > > take some time. In the meantime I'm attaching v11 of the patch that

Re: Online checksums verification in the backend

2020-09-09 Thread Julien Rouhaud
On Wed, Sep 09, 2020 at 11:25:29AM +0200, Julien Rouhaud wrote: > > I'll do some becnhmarking and see if I can get some figures, but it'll > probably > take some time. In the meantime I'm attaching v11 of the patch that should > address all other comments. I just realized that I forgot to

Re: Online checksums verification in the backend

2020-09-09 Thread Julien Rouhaud
On Wed, Sep 9, 2020 at 2:37 PM Michael Paquier wrote: > > Another thing that was itching me is the introduction of 3 GUCs with > one new category for the sake of two SQL functions. For VACUUM we > have many things relying on the GUC delays, with autovacuum and manual > vacuum. Perhaps it would

Re: Online checksums verification in the backend

2020-09-09 Thread Michael Paquier
On Wed, Sep 09, 2020 at 11:25:24AM +0200, Julien Rouhaud wrote: > I assumed that the odds of having to check the buffer twice were so low, and > avoiding to keep a bufmapping lock while doing some IO was an uncontroversial > enough optimisation, but maybe that's only wishful thinking. Perhaps it

Re: Online checksums verification in the backend

2020-09-09 Thread Julien Rouhaud
On Mon, Sep 07, 2020 at 05:50:38PM +0900, Michael Paquier wrote: > On Mon, Sep 07, 2020 at 09:38:30AM +0200, Julien Rouhaud wrote: > > Did you mean creating a new checksumfuncs.c file? I don't find any > > such file in the current tree. > > Your patch adds checksumfuncs.c, so the subroutines

Re: Online checksums verification in the backend

2020-09-07 Thread Michael Paquier
On Tue, Sep 08, 2020 at 11:36:45AM +0900, Masahiko Sawada wrote: > On Mon, 7 Sep 2020 at 15:59, Michael Paquier wrote: >> In this case it could be better to unregister from the >> CF app for this entry. > > Well, I sent review comments on this patch and Julien fixed all > comments. So I’d

Re: Online checksums verification in the backend

2020-09-07 Thread Masahiko Sawada
On Mon, 7 Sep 2020 at 15:59, Michael Paquier wrote: > > On Tue, Jul 14, 2020 at 11:08:08AM +0200, Julien Rouhaud wrote: > > On Sun, Jul 12, 2020 at 12:34:03PM -0500, Justin Pryzby wrote: > >> Small language fixes in comments and user-facing docs. > > > > Thanks a lot! I just fixed a small issue

Re: Online checksums verification in the backend

2020-09-07 Thread Michael Paquier
On Mon, Sep 07, 2020 at 09:38:30AM +0200, Julien Rouhaud wrote: > Did you mean creating a new checksumfuncs.c file? I don't find any > such file in the current tree. Your patch adds checksumfuncs.c, so the subroutines grabbing a given block could just be moved there. > I'm not sure I understand.

Re: Online checksums verification in the backend

2020-09-07 Thread Julien Rouhaud
On Mon, Sep 7, 2020 at 8:59 AM Michael Paquier wrote: > > +#include "postgres.h" > + > +#include "access/tupdesc.h" > +#include "common/relpath.h" > #include "storage/block.h" > +#include "utils/relcache.h" > +#include "utils/tuplestore.h" > [...] > +extern bool check_one_block(Relation

Re: Online checksums verification in the backend

2020-09-07 Thread Michael Paquier
On Tue, Jul 14, 2020 at 11:08:08AM +0200, Julien Rouhaud wrote: > On Sun, Jul 12, 2020 at 12:34:03PM -0500, Justin Pryzby wrote: >> Small language fixes in comments and user-facing docs. > > Thanks a lot! I just fixed a small issue (see below), PFA updated v10. Sawada-san, you are registered as

Re: Online checksums verification in the backend

2020-07-14 Thread Julien Rouhaud
On Sun, Jul 12, 2020 at 12:34:03PM -0500, Justin Pryzby wrote: > Small language fixes in comments and user-facing docs. Thanks a lot! I just fixed a small issue (see below), PFA updated v10. > > diff --git a/src/backend/storage/page/checksum.c > b/src/backend/storage/page/checksum.c > index

Re: Online checksums verification in the backend

2020-07-12 Thread Justin Pryzby
Small language fixes in comments and user-facing docs. diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 88efb38556..39596db193 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -26162,7 +26162,7 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'),

Re: Online checksums verification in the backend

2020-07-05 Thread Daniel Gustafsson
> On 5 Apr 2020, at 13:17, Julien Rouhaud wrote: > On Sun, Apr 05, 2020 at 08:01:36PM +0900, Masahiko Sawada wrote: >> Thank you for updating the patch! The patch looks good to me. >> >> I've marked this patch as Ready for Committer. I hope this patch will >> get committed to PG13. > Thanks a

Re: Online checksums verification in the backend

2020-04-05 Thread Julien Rouhaud
On Sun, Apr 05, 2020 at 08:01:36PM +0900, Masahiko Sawada wrote: > On Sun, 5 Apr 2020 at 18:45, Julien Rouhaud wrote: > > > > On Sun, Apr 05, 2020 at 06:08:06PM +0900, Masahiko Sawada wrote: > > > > > > Why do we need two rows in the doc? For instance, replication slot > > > functions have some

Re: Online checksums verification in the backend

2020-04-05 Thread Masahiko Sawada
On Sun, 5 Apr 2020 at 18:45, Julien Rouhaud wrote: > > On Sun, Apr 05, 2020 at 06:08:06PM +0900, Masahiko Sawada wrote: > > > > Why do we need two rows in the doc? For instance, replication slot > > functions have some optional arguments but there is only one row in > > the doc. So I think we

Re: Online checksums verification in the backend

2020-04-05 Thread Julien Rouhaud
On Sun, Apr 05, 2020 at 06:08:06PM +0900, Masahiko Sawada wrote: > > Why do we need two rows in the doc? For instance, replication slot > functions have some optional arguments but there is only one row in > the doc. So I think we don't need to change the doc from the previous > version patch. >

Re: Online checksums verification in the backend

2020-04-05 Thread Masahiko Sawada
On Sun, 5 Apr 2020 at 17:44, Julien Rouhaud wrote: > > On Sun, Apr 05, 2020 at 01:13:30PM +0900, Masahiko Sawada wrote: > > > > Thank you for updating the patch! The patch looks good to me. Here are > > some random comments mostly about cosmetic changes. > > > > Thanks a lot for the review!

Re: Online checksums verification in the backend

2020-04-05 Thread Julien Rouhaud
On Sun, Apr 05, 2020 at 01:13:30PM +0900, Masahiko Sawada wrote: > > Thank you for updating the patch! The patch looks good to me. Here are > some random comments mostly about cosmetic changes. > Thanks a lot for the review! > > 1. > I think we can have two separate SQL functions: >

Re: Online checksums verification in the backend

2020-04-04 Thread Masahiko Sawada
On Sat, 4 Apr 2020 at 18:04, Julien Rouhaud wrote: > > On Fri, Apr 03, 2020 at 11:39:11AM +0200, Julien Rouhaud wrote: > > On Fri, Apr 03, 2020 at 12:24:50PM +0900, Masahiko Sawada wrote: > > > > > > check_relation_fork() seems to quite depends on pg_check_relation() > > > because the returned

Re: Online checksums verification in the backend

2020-04-04 Thread Julien Rouhaud
On Fri, Apr 03, 2020 at 11:39:11AM +0200, Julien Rouhaud wrote: > On Fri, Apr 03, 2020 at 12:24:50PM +0900, Masahiko Sawada wrote: > > > > check_relation_fork() seems to quite depends on pg_check_relation() > > because the returned tuplestore is specified by pg_check_relation(). > > It's just an

Re: Online checksums verification in the backend

2020-04-03 Thread Julien Rouhaud
On Fri, Apr 03, 2020 at 12:24:50PM +0900, Masahiko Sawada wrote: > On Sat, 28 Mar 2020 at 21:19, Julien Rouhaud wrote: > > > The current patch still checks SearchSysCacheExists1() before > relation_open. Why do we need to call SearchSysCacheExists1() here? I > think if the given relation doesn't

Re: Online checksums verification in the backend

2020-04-02 Thread Masahiko Sawada
On Sat, 28 Mar 2020 at 21:19, Julien Rouhaud wrote: > > On Sat, Mar 28, 2020 at 12:28:27PM +0900, Masahiko Sawada wrote: > > On Wed, 18 Mar 2020 at 19:11, Julien Rouhaud wrote: > > > > > > v5 attached > > > > Thank you for updating the patch. I have some comments: > > Thanks a lot for the

Re: Online checksums verification in the backend

2020-03-28 Thread Julien Rouhaud
On Sat, Mar 28, 2020 at 12:28:27PM +0900, Masahiko Sawada wrote: > On Wed, 18 Mar 2020 at 19:11, Julien Rouhaud wrote: > > > > v5 attached > > Thank you for updating the patch. I have some comments: Thanks a lot for the review! > 1. > + > +pg_check_relation(relation > oid, fork

Re: Online checksums verification in the backend

2020-03-27 Thread Masahiko Sawada
On Wed, 18 Mar 2020 at 19:11, Julien Rouhaud wrote: > > On Wed, Mar 18, 2020 at 07:06:19AM +0100, Julien Rouhaud wrote: > > On Wed, Mar 18, 2020 at 01:20:47PM +0900, Michael Paquier wrote: > > > On Mon, Mar 16, 2020 at 09:21:22AM +0100, Julien Rouhaud wrote: > > > > On Mon, Mar 16, 2020 at

Re: Online checksums verification in the backend

2020-03-18 Thread Julien Rouhaud
On Wed, Mar 18, 2020 at 07:06:19AM +0100, Julien Rouhaud wrote: > On Wed, Mar 18, 2020 at 01:20:47PM +0900, Michael Paquier wrote: > > On Mon, Mar 16, 2020 at 09:21:22AM +0100, Julien Rouhaud wrote: > > > On Mon, Mar 16, 2020 at 12:29:28PM +0900, Michael Paquier wrote: > > >> With a large amount

Re: Online checksums verification in the backend

2020-03-18 Thread Julien Rouhaud
On Wed, Mar 18, 2020 at 01:20:47PM +0900, Michael Paquier wrote: > On Mon, Mar 16, 2020 at 09:21:22AM +0100, Julien Rouhaud wrote: > > On Mon, Mar 16, 2020 at 12:29:28PM +0900, Michael Paquier wrote: > >> Based on the feedback gathered on this thread, I guess that you should > >> have a SRF

Re: Online checksums verification in the backend

2020-03-18 Thread Julien Rouhaud
On Wed, Mar 18, 2020 at 01:20:47PM +0900, Michael Paquier wrote: > On Mon, Mar 16, 2020 at 09:21:22AM +0100, Julien Rouhaud wrote: > > On Mon, Mar 16, 2020 at 12:29:28PM +0900, Michael Paquier wrote: > >> With a large amount of > >> shared buffer eviction you actually increase the risk of torn

Re: Online checksums verification in the backend

2020-03-17 Thread Michael Paquier
On Mon, Mar 16, 2020 at 09:21:22AM +0100, Julien Rouhaud wrote: > On Mon, Mar 16, 2020 at 12:29:28PM +0900, Michael Paquier wrote: >> With a large amount of >> shared buffer eviction you actually increase the risk of torn page >> reads. Instead of a logic relying on partition mapping locks, which

Re: Online checksums verification in the backend

2020-03-16 Thread Julien Rouhaud
On Mon, Mar 16, 2020 at 02:15:27PM +0100, Julien Rouhaud wrote: > On Mon, Mar 16, 2020 at 09:42:39AM +0100, Julien Rouhaud wrote: > > On Mon, Mar 16, 2020 at 01:53:35PM +0900, Masahiko Sawada wrote: > > > > > > In addition to comments from Michael-san, here are my comments: > > Thanks both for the

Re: Online checksums verification in the backend

2020-03-16 Thread Julien Rouhaud
On Mon, Mar 16, 2020 at 09:42:39AM +0100, Julien Rouhaud wrote: > On Mon, Mar 16, 2020 at 01:53:35PM +0900, Masahiko Sawada wrote: > > > > In addition to comments from Michael-san, here are my comments: Thanks both for the reviews. I'm attaching a v3 with all comments addressed, except: > It

Re: Online checksums verification in the backend

2020-03-16 Thread Julien Rouhaud
On Mon, Mar 16, 2020 at 01:53:35PM +0900, Masahiko Sawada wrote: > > In addition to comments from Michael-san, here are my comments: > > 1. > + if (!is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_SERVER_FILES)) > + ereport(ERROR, > +

Re: Online checksums verification in the backend

2020-03-16 Thread Julien Rouhaud
Thanks for the review Michael! On Mon, Mar 16, 2020 at 12:29:28PM +0900, Michael Paquier wrote: > On Wed, Mar 11, 2020 at 08:18:23AM +0100, Julien Rouhaud wrote: > > The cfbot reported a build failure, so here's a rebased v2 which also > > contains > > the pg_stat_report_failure() call and extra

Re: Online checksums verification in the backend

2020-03-15 Thread Masahiko Sawada
On Wed, 11 Mar 2020 at 16:18, Julien Rouhaud wrote: > > On Tue, Dec 10, 2019 at 11:12:34AM +0100, Julien Rouhaud wrote: > > On Tue, Dec 10, 2019 at 3:26 AM Michael Paquier wrote: > > > > > > On Mon, Dec 09, 2019 at 07:02:43PM +0100, Julien Rouhaud wrote: > > > > On Mon, Dec 9, 2019 at 5:21 PM

Re: Online checksums verification in the backend

2020-03-15 Thread Michael Paquier
On Wed, Mar 11, 2020 at 08:18:23AM +0100, Julien Rouhaud wrote: > The cfbot reported a build failure, so here's a rebased v2 which also contains > the pg_stat_report_failure() call and extra log info. + * - if a block is not found in shared_buffers, the LWLock is relased and the + * block is

Re: Online checksums verification in the backend

2020-03-11 Thread Julien Rouhaud
On Tue, Dec 10, 2019 at 11:12:34AM +0100, Julien Rouhaud wrote: > On Tue, Dec 10, 2019 at 3:26 AM Michael Paquier wrote: > > > > On Mon, Dec 09, 2019 at 07:02:43PM +0100, Julien Rouhaud wrote: > > > On Mon, Dec 9, 2019 at 5:21 PM Robert Haas wrote: > > >> Some people might prefer notices,

Re: Online checksums verification in the backend

2019-12-24 Thread Masahiko Sawada
On Tue, 24 Dec 2019 at 16:09, Julien Rouhaud wrote: > > On Tue, Dec 24, 2019 at 4:23 AM Masahiko Sawada wrote: > > > > On Fri, Dec 6, 2019 at 11:51 PM Julien Rouhaud wrote: > > > > > > This brings the second consideration: how to report the list corrupted > > > blocks to end users. As I said

Re: Online checksums verification in the backend

2019-12-23 Thread Julien Rouhaud
On Tue, Dec 24, 2019 at 4:23 AM Masahiko Sawada wrote: > > On Fri, Dec 6, 2019 at 11:51 PM Julien Rouhaud wrote: > > > > This brings the second consideration: how to report the list corrupted > > blocks to end users. As I said this is for now returned via the SRF, > > but this is clearly not

Re: Online checksums verification in the backend

2019-12-23 Thread Masahiko Sawada
On Fri, Dec 6, 2019 at 11:51 PM Julien Rouhaud wrote: > > Hi, > > This topic was discussed several times, with the most recent > discussions found at [1] and [2]. Based on those discussions, my > understanding is that the current approach in BASE_BACKUP has too many > drawbacks and we should

Re: Online checksums verification in the backend

2019-12-10 Thread Julien Rouhaud
On Tue, Dec 10, 2019 at 3:26 AM Michael Paquier wrote: > > On Mon, Dec 09, 2019 at 07:02:43PM +0100, Julien Rouhaud wrote: > > On Mon, Dec 9, 2019 at 5:21 PM Robert Haas wrote: > >> Some people might prefer notices, because you can get those while the > >> thing is still running, rather than a

Re: Online checksums verification in the backend

2019-12-09 Thread Michael Paquier
On Mon, Dec 09, 2019 at 07:02:43PM +0100, Julien Rouhaud wrote: > On Mon, Dec 9, 2019 at 5:21 PM Robert Haas wrote: >> Some people might prefer notices, because you can get those while the >> thing is still running, rather than a result set, which you will only >> see when the query finishes.

Re: Online checksums verification in the backend

2019-12-09 Thread Julien Rouhaud
On Mon, Dec 9, 2019 at 5:21 PM Robert Haas wrote: > > On Fri, Dec 6, 2019 at 9:51 AM Julien Rouhaud wrote: > > > This brings the second consideration: how to report the list corrupted > > blocks to end users. As I said this is for now returned via the SRF, > > but this is clearly not ideal and

Re: Online checksums verification in the backend

2019-12-09 Thread Robert Haas
On Fri, Dec 6, 2019 at 9:51 AM Julien Rouhaud wrote: > This topic was discussed several times, with the most recent > discussions found at [1] and [2]. Based on those discussions, my > understanding is that the current approach in BASE_BACKUP has too many > drawbacks and we should instead do

Online checksums verification in the backend

2019-12-06 Thread Julien Rouhaud
Hi, This topic was discussed several times, with the most recent discussions found at [1] and [2]. Based on those discussions, my understanding is that the current approach in BASE_BACKUP has too many drawbacks and we should instead do this check in the backend. I've been working using such