From: Nathan Bossart <nathandbossart@gmail.com>
To: Sravan Kumar <sravanvcybage@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: bharath.rupireddyforpostgres@gmail.com
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Question regarding "Make archiver process an auxiliary process. commit"
Date: Fri, 20 Jan 2023 11:39:56 -0800
Message-ID: <20230120193956.GA2831@nathanxps13> (raw)
In-Reply-To: <CA+=NbjhEx-dZx=xqbj30ym-zQg9HygEYRdYdwZC9a471Wv92_w@mail.gmail.com>
References: <CA+=NbjiGSTqNE=njikNrX1=R=SXGaxV-A22qZ2nH8=O0B6SSAg@mail.gmail.com>
<CALj2ACV3HsLWTwQDfdS6w-8_FfpPpRE5WYJ7-At7pk7K_7HuaQ@mail.gmail.com>
<CA+=Nbjhxyj54hs4TvdPmRsGfxWLQPagzPQysOGKMP5sMo6aLFw@mail.gmail.com>
<20221207.151939.662411275254345514.horikyota.ntt@gmail.com>
<CA+=NbjhEx-dZx=xqbj30ym-zQg9HygEYRdYdwZC9a471Wv92_w@mail.gmail.com>
On Wed, Jan 04, 2023 at 11:35:33AM +0530, Sravan Kumar wrote:
> I have added the thread to the commitfest: https://commitfest.postgresql.org/42/
> Did you get a chance to review the patch? Please let me know if you
> need anything from my end.
This seems like worthwhile simplification to me. Ultimately, your patch
shouldn't result in any sort of signficant behavior change, and I don't see
any reason to further complicate the timeout calculation. The copy loop
will run any time the archiver's latch is set, and it'll wait up to 60
seconds otherwise. As discussed upthread, it might be possible to remove
the timeout completely, but that probably deserves its own thread.
I noticed that time.h is no longer needed by the archiver, so I removed
that and fixed an indentation nitpick in the attached v2. I'm going to set
the commitfest entry to ready-for-committer shortly after sending this
message.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.comAttachments:
[text/x-diff] v2-0001-simplify-wait-loop-in-the-archiver.patch (1.9K, ../20230120193956.GA2831@nathanxps13/2-v2-0001-simplify-wait-loop-in-the-archiver.patch)
download | inline diff:
From a06609e839f039b7e7806456eaf4ee113cfabc3c Mon Sep 17 00:00:00 2001
From: Sravan Velagandula <sravan.velagandula@enterprisedb.com>
Date: Tue, 6 Dec 2022 06:21:38 -0500
Subject: [PATCH v2 1/1] simplify wait loop in the archiver
---
src/backend/postmaster/pgarch.c | 29 +++++++++--------------------
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.cindex 8ecdb9ca23..6e28067596 100644--- a/src/backend/postmaster/pgarch.c+++ b/src/backend/postmaster/pgarch.c@@ -25,7 +25,6 @@
*/
#include "postgres.h"
-#include <time.h>
#include <sys/stat.h>
#include <unistd.h>
@@ -297,7 +296,6 @@ pgarch_waken_stop(SIGNAL_ARGS)
static void
pgarch_MainLoop(void)
{
- pg_time_t last_copy_time = 0;
bool time_to_stop;
/*
@@ -335,30 +333,21 @@ pgarch_MainLoop(void)
/* Do what we're here for */
pgarch_ArchiverCopyLoop();
- last_copy_time = time(NULL);
/*
* Sleep until a signal is received, or until a poll is forced by
- * PGARCH_AUTOWAKE_INTERVAL having passed since last_copy_time, or- * until postmaster dies.+ * PGARCH_AUTOWAKE_INTERVAL, or until postmaster dies.
*/
if (!time_to_stop) /* Don't wait during last iteration */
{
- pg_time_t curtime = (pg_time_t) time(NULL);- int timeout;-- timeout = PGARCH_AUTOWAKE_INTERVAL - (curtime - last_copy_time);- if (timeout > 0)- {- int rc;-- rc = WaitLatch(MyLatch,- WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,- timeout * 1000L,- WAIT_EVENT_ARCHIVER_MAIN);- if (rc & WL_POSTMASTER_DEATH)- time_to_stop = true;- }+ int rc;++ rc = WaitLatch(MyLatch,+ WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,+ PGARCH_AUTOWAKE_INTERVAL * 1000L,+ WAIT_EVENT_ARCHIVER_MAIN);+ if (rc & WL_POSTMASTER_DEATH)+ time_to_stop = true;
}
/*
--
2.25.1
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: nathandbossart@gmail.com, sravanvcybage@gmail.com, horikyota.ntt@gmail.com, bharath.rupireddyforpostgres@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Question regarding "Make archiver process an auxiliary process. commit"
In-Reply-To: <20230120193956.GA2831@nathanxps13>
* 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