Date: Fri, 27 Dec 2019 16:46:18 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: craig@2ndquadrant.com, tomas.vondra@2ndquadrant.com,
	pgsql-hackers@postgresql.org
Subject: Re: xact_start for walsender & logical decoding not updated
Message-ID: <20191227194618.GA30125@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="BXVAT5kNtrzKuDFl"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20191213.150124.69303805839083319.horikyota.ntt@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


--BXVAT5kNtrzKuDFl
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2019-Dec-13, Kyotaro Horiguchi wrote:

> At Fri, 13 Dec 2019 13:05:41 +0800, Craig Ringer <craig@2ndquadrant.com> wrote in 
> > On Wed, 11 Dec 2019 at 02:08, Alvaro Herrera <alvherre@2ndquadrant.com>
> > wrote:
> > 
> > > On 2019-Dec-10, Tomas Vondra wrote:
> > >
> > > > On Tue, Dec 10, 2019 at 09:42:17AM +0900, Kyotaro Horiguchi wrote:
> > > > > At Tue, 10 Dec 2019 00:44:09 +0100, Tomas Vondra <
> > > tomas.vondra@2ndquadrant.com> wrote in
> > >
> > > > > I'm not sure how much xact_start for walsender is useful and we really
> > > > > is not running a statement there.  Also autovac launcher starts
> > > > > transaction without a valid statement timestamp perhaps for the same
> > > > > reason.
> > > >
> > > > Maybe, but then maybe we should change it so that we don't report any
> > > > timestamps for such processes.
> > >
> > > Yeah, I think we should to that.
> > Agreed. Don't report a transaction start timestamp at all if we're not in a
> > read/write txn in the walsender, which we should never be when using a
> > historic snapshot.
> > 
> > It's not interesting or relevant.

This patch changes xact.c to avoid updating transaction start timestamps
for walsenders (maybe more commentary is desirable).  I think logical
decoding is just a special form of walsender and thus it would also be
updated by this patch, unless I misunderstood what Tomas explained.

> > Reporting the commit timestamp of the current or last-processed xact would
> > likely just be confusing. I'd rather see that in pg_stat_replication if
> > we're going to show it, that way we can label it usefully.
> 
> Sounds reasonable.

Developers interested in this feature can submit a patch, as usual :-)

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

--BXVAT5kNtrzKuDFl
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="walsender-stmtts.patch"

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 5353b6ab0b..63652ab19f 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -816,6 +816,10 @@ GetCurrentTransactionStopTimestamp(void)
 void
 SetCurrentStatementStartTimestamp(void)
 {
+	/* walsenders don't want to have stmt timestamps */
+	if (am_walsender)
+		return;
+
 	if (!IsParallelWorker())
 		stmtStartTimestamp = GetCurrentTimestamp();
 	else

--BXVAT5kNtrzKuDFl--