Re: archive status ".ready" files may be created too early

From: "alvherre(at)alvh(dot)no-ip(dot)org" <alvherre(at)alvh(dot)no-ip(dot)org>
To: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "x4mmm(at)yandex-team(dot)ru" <x4mmm(at)yandex-team(dot)ru>, "a(dot)lubennikova(at)postgrespro(dot)ru" <a(dot)lubennikova(at)postgrespro(dot)ru>, "hlinnaka(at)iki(dot)fi" <hlinnaka(at)iki(dot)fi>, "matsumura(dot)ryo(at)fujitsu(dot)com" <matsumura(dot)ryo(at)fujitsu(dot)com>, "masao(dot)fujii(at)gmail(dot)com" <masao(dot)fujii(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: archive status ".ready" files may be created too early
Date: 2021-08-23 15:49:27
Message-ID: 202108231549.2zhbgjz2hikp@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2021-Aug-21, Bossart, Nathan wrote:

> > Well, the thing I realized is that these three helper functions have
> > exactly one caller each. I think the compiler is going to inline them,
> > so there isn't going to be a function call in the assembly. I haven't
> > verified this, though.
>
> Good point. It looks like they're getting inlined for me.

I still didn't like it, because it looks like we're creating an API for
which there can be only one caller. So I expanded the functions in the
caller. It doesn't look too bad. However ...

... while reading the resulting code after backpatching to all branches,
I realized that if there are no registrations whatsoever, then archiving
won't do anything, which surely is the wrong thing to do. The correct
behavior should be "if there are no registrations, then *all* flushed
segments can be notified".

I'll fix that ...

Another thing I didn't like is that you used a name ending in RecPtr for
the LSN, which gives no indication that it really is the *end* LSN, not
the start pointer. And it won't play nice with the need to add the
*start* LSN which we'll need to implement solving the equivalent problem
for streaming replication. I'll rename those to
earliestSegBoundaryEndPtr and latestSegBoundaryEndPtr.

--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
Syntax error: function hell() needs an argument.
Please choose what hell you want to involve.

Attachment Content-Type Size
v15-0001-Avoid-creating-archive-status-.ready-files-too-e.patch text/x-diff 18.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bossart, Nathan 2021-08-23 15:50:29 Re: .ready and .done files considered harmful
Previous Message Tom Lane 2021-08-23 15:43:07 Regexp: observable bug from careless usage of zaptreesubs