pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Bossart, Nathan <bossartn@amazon.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: x4mmm@yandex-team.ru <x4mmm@yandex-team.ru>
Cc: a.lubennikova@postgrespro.ru <a.lubennikova@postgrespro.ru>
Cc: hlinnaka@iki.fi <hlinnaka@iki.fi>
Cc: matsumura.ryo@fujitsu.com <matsumura.ryo@fujitsu.com>
Cc: masao.fujii@gmail.com <masao.fujii@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: archive status ".ready" files may be created too early
Date: Sat, 31 Jul 2021 17:53:02 +0000
Message-ID: <997F25DB-5D57-4FB1-BA97-6110D8BF1415@amazon.com> (raw)
In-Reply-To: <202107311610.jbpvq7kgmwc3@alvherre.pgsql>
References: <7782D697-3F76-4E2A-A223-BA6E2B91A7FD@amazon.com>
	<202107311610.jbpvq7kgmwc3@alvherre.pgsql>

On 7/31/21, 9:12 AM, "Alvaro Herrera" <alvherre@alvh.no-ip.org> wrote:
> On 2021-Jul-31, Bossart, Nathan wrote:
>
>> On 7/30/21, 4:52 PM, "Alvaro Herrera" <alvherre@alvh.no-ip.org> wrote:
>
>> > I noticed that XLogCtl->lastNotifiedSeg is protected by both the
>> > info_lck and ArchNotifyLock.  I think it it's going to be protected by
>> > the lwlock, then we should drop the use of the spinlock.
>>
>> That seems reasonable to me.  This means that the lock is acquired at
>> the end of every XLogWrite(),
>
> Uhh, actually that there sounds really bad; it's going to be a serious
> contention point.
>
> Another option might be to make it an atomic.

This is why I was trying to get away with just using info_lck for
reading lastNotifiedSeg.  ArchNotifyLock is mostly intended to protect
RecordBoundaryMap.  However, since lastNotifiedSeg is used in
functions like GetLatestRecordBoundarySegment() that access the map, I
found it easier to reason about things if I knew that it wouldn't
change as long as I held ArchNotifyLock.

I think the main downside of making lastNotifiedSeg an atomic is that
the value we first read in NotifySegmentsReadyForArchive() might not
be up-to-date, which means we might hold off creating .ready files
longer than necessary.

Nathan



view thread (127+ messages)  latest in thread

Message-ID: <997F25DB-5D57-4FB1-BA97-6110D8BF1415@amazon.com>
Permalink:  ../997F25DB-5D57-4FB1-BA97-6110D8BF1415@amazon.com/
Also on:    postgresql.org/message-id/997F25DB-5D57-4FB1-BA97-6110D8BF1415@amazon.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: bossartn@amazon.com, alvherre@alvh.no-ip.org, horikyota.ntt@gmail.com, x4mmm@yandex-team.ru, a.lubennikova@postgrespro.ru, hlinnaka@iki.fi, matsumura.ryo@fujitsu.com, masao.fujii@gmail.com
  Subject: Re: archive status ".ready" files may be created too early
  In-Reply-To: <997F25DB-5D57-4FB1-BA97-6110D8BF1415@amazon.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox