On 2021/03/05 17:18, Kyotaro Horiguchi wrote:
At Thu, 21 Jan 2021 12:03:48 +0900 (JST), Kyotaro Horiguchi
<horikyota....@gmail.com> wrote in
Commit 960869da08 (database statistics) conflicted with this. Rebased.
I'm concerned about the behavior that pgstat_update_connstats calls
GetCurrentTimestamp() every time stats update happens (with intervals
of 10s-60s in this patch). But I didn't change that design since that
happens with about 0.5s intervals in master and the rate is largely
reduced in this patch, to make this patch simpler.
I stepped on my foot, and another commit coflicted. Just rebased.
Thanks for rebasing the patches!
I think that 0003 patch is self-contained and useful, for example which
enables us to monitor archiver process in pg_stat_activity. So IMO
it's worth pusing 0003 patch firstly.
Here are the review comments for 0003 patch.
+ /* Archiver process's latch */
+ Latch *archiverLatch;
+ /* Current shared estimate of appropriate spins_per_delay value */
The last line in the above seems not necessary.
In proc.h, NUM_AUXILIARY_PROCS needs to be incremented.
/* ----------
* Functions called from postmaster
* ----------
*/
extern int pgarch_start(void);
In pgarch.h, the above is not necessary.
+extern void XLogArchiveWakeup(void);
This seems no longer necessary.
+extern void XLogArchiveWakeupStart(void);
+extern void XLogArchiveWakeupEnd(void);
+extern void XLogArchiveWakeup(void);
These seem also no longer necessary.
PgArchPID = 0;
if (!EXIT_STATUS_0(exitstatus))
- LogChildExit(LOG, _("archiver process"),
- pid, exitstatus);
- if (PgArchStartupAllowed())
- PgArchPID = pgarch_start();
+ HandleChildCrash(pid, exitstatus,
+ _("archiver
process"));
I don't think that we should treat non-zero exit condition as a crash,
as before. Otherwise when archive_command fails on a signal,
archiver emits FATAL error and which leads the server restart.
- * walwriter, autovacuum, or background worker.
+ * walwriter, autovacuum, archiver or background worker.
*
* The objectives here are to clean up our local state about the child
* process, and to signal all other remaining children to quickdie.
@@ -3609,6 +3606,18 @@ HandleChildCrash(int pid, int exitstatus, const char
*procname)
signal_child(AutoVacPID, (SendStop ? SIGSTOP : SIGQUIT));
}
+ /* Take care of the archiver too */
+ if (pid == PgArchPID)
+ PgArchPID = 0;
+ else if (PgArchPID != 0 && take_action)
+ {
+ ereport(DEBUG2,
+ (errmsg_internal("sending %s to process %d",
+ (SendStop ? "SIGSTOP" :
"SIGQUIT"),
+ (int)
PgArchPID)));
+ signal_child(PgArchPID, (SendStop ? SIGSTOP : SIGQUIT));
+ }
+
Same as above.
In xlogarchive.c, "#include "storage/pmsignal.h"" is no longer necessary.
pgarch_forkexec() should be removed from pgarch.c because it's no longer used.
/* ------------------------------------------------------------
* Public functions called from postmaster follow
* ------------------------------------------------------------
*/
The definition of PgArchiverMain() should be placed just
after the above comment.
exit(0) in PgArchiverMain() should be proc_exit(0)?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
