Re: pg_waldump stucks with options --follow or -f and --stats or -z

On Sat, Nov 20, 2021 at 10:03:27AM +0530, Bharath Rupireddy wrote:
> Here's the v2 patch with the above changes i.e. pg_waldump now eimits
> the computed stats at the time of termination. Please review it.

+#define MAX_XLINFO_TYPES 16
+#define fatal_error(...) do { pg_log_fatal(__VA_ARGS__);
exit(EXIT_FAILURE); } while(0)
typedef struct XLogDumpPrivate
Moving around those declarations does not relate to this patch, so I
would recommend to leave this out to ease reviews.

+ /*
+ * Although the pg_free calls (above and below) are unnecessary here on the
+ * process exit, let's not leak any memory knowingly.
+ */
+ pg_free(opts);
+
+ exit(EXIT_FAILURE);
There is no need to care about the two pg_free() calls here, so let's
remove all that.

+ pg_free(opts);
+ pg_free(stats);
+
return EXIT_SUCCESS;
[...]
bad_argument:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
progname);
+ pg_free(opts);
return EXIT_FAILURE;
Same here.

+static void
+SignalHandlerForTermination(int signum)
+{
+ /* Display the stats only if they are valid */
+ if (stats &&
+ !XLogRecPtrIsInvalid(stats_startptr) &&
+ !XLogRecPtrIsInvalid(stats_endptr))
+ {
+ XLogDumpDisplayStats();
+ pg_free(stats);
+ }
+
+ /*
+ * Although the pg_free calls (above and below) are unnecessary here on the
+ * process exit, let's not leak any memory knowingly.
+ */
+ pg_free(opts);
+
+ exit(EXIT_FAILURE);
+}
Doing the dump of the stats followed by the exit() within the signal
handler callback is incorrect. The only safe thing that can be set
and manipulated in a signal handler is roughly a sig_atomic_t flag.
What you should do is to set such a flag in the signal handler, and
then check its value in the main loop of pg_waldump, dumping the stats
if it is detected as turned on because of a signal.
--
Michael