Date: Fri, 29 Jan 2016 09:13:30 +0100
From: Andres Freund <andres@anarazel.de>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>,
 Robert Haas <robertmhaas@gmail.com>,
 Jeff Janes <jeff.janes@gmail.com>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: Re: BUG #13685: Archiving while idle every
 archive_timeout with wal_level hot_standby
Message-ID: <20160129081330.e5mj65du2cknxsja@alap3.anarazel.de>
References: 
 <CAA4eK1K5j-7iM_MXduO4NFWqBJ3xru3_=xifFg2kxr97jkSTLw@mail.gmail.com>
 <CAB7nPqRnVx6VB_P5bc1tLJoN75L2SLTOC26ESuwmbZ0tCbE2sQ@mail.gmail.com>
 <CAA4eK1K0cRkChor7Wv43Bn76Pz2NgAeQuY=n93Oed7a-veM68Q@mail.gmail.com>
 <CAB7nPqTA-ByticAAPZz77+RyxuHhTjK8fbp=AScdC0xjCU1zPA@mail.gmail.com>
 <CAA4eK1K6geOYNzVcch-WW9FcMLQDGwOD+OBiK2UKc8Jm4BfRXA@mail.gmail.com>
 <CAB7nPqSez6mVPumK8rQ_b7nmkStnbhPx_hfz=V0H5BLmDBDTwg@mail.gmail.com>
 <CAA4eK1Jj+dXqv4QeSq_t8uX4T2mE-2n+5JWp6NYQjhUCmAJj4g@mail.gmail.com>
 <CAB7nPqQsyyGj8E9951K2EGQF_cy-WUw3pjJLjz8_L7c88Kd-Vw@mail.gmail.com>
 <CAA4eK1+PwJ1z3rr4EyPfsd5+vUVSfjjq_htVNcZ9SvAgSG2PCA@mail.gmail.com>
 <CAB7nPqQ4OGNvOEUgrOwQyxsVotdXa0AeVtKHM0xm+oNy2dtkPA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAB7nPqQ4OGNvOEUgrOwQyxsVotdXa0AeVtKHM0xm+oNy2dtkPA@mail.gmail.com>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 2016-01-28 16:40:13 +0900, Michael Paquier wrote:
> OK, so as a first step and after thinking about the whole for a while,
> I have finished with the patch attached. This patch is aimed at
> avoiding unnecessary checkpoints on idle systems when wal_level >=
> hot_standby by centralizing the check to look at if there has some WAL
> activity since the last checkpoint.

That's not what I suggested.

>  /*
> + * XLOGHasActivity -- Check if XLOG had some significant activity or
> + * if it is idle lately. This is primarily used to check if there has
> + * been some WAL activity since the last checkpoint that occurred on
> + * system to control the generaton of XLOG record related to standbys.
> + */
> +bool
> +XLOGHasActivity(void)
> +{
> +	XLogCtlInsert *Insert = &XLogCtl->Insert;
> +	XLogRecPtr	redo_lsn = ControlFile->checkPointCopy.redo;
> +	uint64		prev_bytepos;
> +
> +	/* Check if any activity has happened since last checkpoint */
> +	SpinLockAcquire(&Insert->insertpos_lck);
> +	prev_bytepos = Insert->PrevBytePos;
> +	SpinLockRelease(&Insert->insertpos_lck);
> +
> +	return XLogBytePosToRecPtr(prev_bytepos) == redo_lsn;
> +}
>

How should this actually should work reliably, given we *want* to have
included a standby snapshot after the last checkpoint?

In CreateCheckPoint() we have
	/*
	 * Here we update the shared RedoRecPtr for future XLogInsert calls; this
	 * must be done while holding all the insertion locks.
	 *
	RedoRecPtr = XLogCtl->Insert.RedoRecPtr = checkPoint.redo;

computing the next redo rec ptr and then
	if (!shutdown && XLogStandbyInfoActive())
		LogStandbySnapshot();
before the final
	XLogRegisterData((char *) (&checkPoint), sizeof(checkPoint));
	recptr = XLogInsert(RM_XLOG_ID,
						shutdown ? XLOG_CHECKPOINT_SHUTDOWN :
						XLOG_CHECKPOINT_ONLINE);

so the above condition doesn't really something we want to rely on. Am I
missing what you're trying to do?

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers