From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
Subject: | Re: Online checksums verification in the backend |
Date: | 2020-10-30 03:58:13 |
Message-ID: | CAOBaU_Yb=PBm1mkZqYrS1KoHA=jtQQOUzc4Ahnz7npwPbKew6w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Oct 30, 2020 at 10:58 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2020-10-30 10:01:08 +0800, Julien Rouhaud wrote:
> > On Fri, Oct 30, 2020 at 2:17 AM Andres Freund <andres(at)anarazel(dot)de> 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? Seriously?
> >
> > The initial implementation had a different approach, reading the buffer once
> > without holding the buffer mapping lock (which could lead to some false
> > positive in some unlikely scenario), and only if a corruption is detected the
> > read is done once again *while holding the buffer mapping lock* to ensure it's
> > not a false positive. Some benchmarking showed that the performance was worse,
> > so we dropped that optimisation. Should we go back to something like that or
> > do you have a better way to ensure a consistent read of a buffer which isn't in
> > shared buffers?
>
> I suspect that you're gonna need something quite different than what the
> function is doing right now. Not because such a method will be faster in
> isolation, but because there's a chance to have it correct and not have
> a significant performance impact onto the rest of the system.
>
> I've not thought about it in detail yet. Is suspect you'll need to
> ensure there is a valid entry in the buffer mapping table for the buffer
> you're processing. By virtue of setting BM_IO_IN_PROGRESS on that entry
> you're going to prevent concurrent IO from starting until your part is
> done.
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.
From | Date | Subject | |
---|---|---|---|
Next Message | Tatsuo Ishii | 2020-10-30 04:06:26 | Re: MINUS SIGN (U+2212) in EUC-JP encoding is mapped to FULLWIDTH HYPHEN-MINUS (U+FF0D) in UTF-8 |
Previous Message | Fujii Masao | 2020-10-30 03:50:59 | Re: document pg_settings view doesn't display custom options |