Date: Tue, 7 Jan 2020 20:52:34 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>, craig@2ndquadrant.com,
	pgsql-hackers@postgresql.org
Subject: Re: xact_start for walsender & logical decoding not updated
Message-ID: <20200107235234.GA21729@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="Qxx1br4bt0+wmkIi"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <30804.1578438763@sss.pgh.pa.us>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


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

On 2020-Jan-07, Tom Lane wrote:

> I wrote:
> > The buildfarm seems less than happy with this.
> 
> ... and, having now looked at the patch, I'm not surprised.
> Breaking stmtStartTimestamp, which is what you did, seems like
> an awfully side-effect-filled route to the goal.  If you want
> to prevent monitoring from showing this, why didn't you just
> prevent monitoring from showing it?  That is, I'd have expected
> some am_walsender logic in or near pgstat.c, not here.

That seems a pretty simple patch; attached (untested).  However, my
patch seemed a pretty decent way to achieve the goal, and I don't
understand why it causes the failure, or indeed why we care about
stmtStartTimestamp at all.  I'll look into this again tomorrow.

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

--Qxx1br4bt0+wmkIi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="walsender-ts.patch"

commit 2808f98edb6889ec602534e070e6a2c660f6377e
Author:     Alvaro Herrera <alvherre@alvh.no-ip.org>
AuthorDate: Tue Jan 7 20:33:28 2020 -0300
CommitDate: Tue Jan 7 20:33:28 2020 -0300

    fix previous

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 0dd6b82f99..9b252213d9 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -758,13 +758,6 @@ GetCurrentTransactionStopTimestamp(void)
 void
 SetCurrentStatementStartTimestamp(void)
 {
-	/*
-	 * Skip if on a walsender; this is not needed, and it confuses monitoring
-	 * if we publish non-NULL values.
-	 */
-	if (am_walsender)
-		return;
-
 	if (!IsParallelWorker())
 		stmtStartTimestamp = GetCurrentTimestamp();
 	else
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 20ce48b2d8..8557684aad 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -732,7 +732,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 			else
 				nulls[7] = true;
 
-			if (beentry->st_xact_start_timestamp != 0)
+			if (beentry->st_xact_start_timestamp != 0 ||
+				beentry->st_backendType != B_WAL_SENDER)
 				values[8] = TimestampTzGetDatum(beentry->st_xact_start_timestamp);
 			else
 				nulls[8] = true;

--Qxx1br4bt0+wmkIi--