Date: Fri, 27 Mar 2020 12:34:02 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: andres@anarazel.de, michael@paquier.xyz, thomas.munro@gmail.com,
	tomas.vondra@2ndquadrant.com, a.zakirov@postgrespro.ru,
	ah@cybertec.at, magnus@hagander.net, robertmhaas@gmail.com,
	tgl@sss.pgh.pa.us, pgsql-hackers@postgresql.org
Subject: Re: shared-memory based stats collector
Message-ID: <20200327153402.GA1045@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200327.163115.1814716981588442968.horikyota.ntt@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Mar-27, Kyotaro Horiguchi wrote:

> +/*
> + * XLogArchiveWakeupEnd - Set up archiver wakeup stuff
> + */
> +void
> +XLogArchiveWakeupStart(void)
> +{
> +	Latch *old_latch PG_USED_FOR_ASSERTS_ONLY;
> +
> +	SpinLockAcquire(&XLogCtl->info_lck);
> +	old_latch = XLogCtl->archiverWakeupLatch;
> +	XLogCtl->archiverWakeupLatch = MyLatch;
> +	SpinLockRelease(&XLogCtl->info_lck);
> +	Assert (old_latch == NULL);
> +}

Comment is wrong about the function name; OTOH I don't think the
old_latch assigment in the fourth line won't work well in non-assert
builds.  But why do you need those shenanigans?  Surely
"Assert(XLogCtl->archiverWakeupLatch == NULL)" in the locked region
before assigning MyLatch should be sufficient and acceptable?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services