Re: Add jsonlog log_destination for JSON server logs

From: Sehrope Sarkuni <sehrope(at)jackdb(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, david(at)fetter(dot)org
Subject: Re: Add jsonlog log_destination for JSON server logs
Date: 2021-09-16 21:27:20
Message-ID: CAH7T-apxdwf7=CHD_Vuo+QgtV6TEnLc0N0_SMTJ-a-jFdOBOvA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Attached three patches refactor the syslogger handling of file based
destinations a bit ... and then a lot.

First patch adds a new constant to represent both file based destinations.
This should make it easier to ensure additional destinations are handled in
"For all file destinations..." situations (e.g. when we add the jsonlog
destination).

Second patch refactors the file descriptor serialization and re-opening in
EXEC_BACKEND forking. Previously the code was duplicated for both stderr
and csvlog. Again, this should simplify adding new destinations as they'd
just invoke the same helper. There's an existing comment about not handling
failed opens in syslogger_parseArgs(...) and this patch doesn't fix that,
but it does provide a single location to do so in the future.

The third patch adds a new internal (to syslogger.c) structure,
FileLogDestination, for file based log destinations and modifies the
existing handling for syslogFile and csvlogFile to defer to a bunch of
helper functions. It also renames "syslogFile" to "stderr_file_info".
Working through this patch, it was initially confusing that the stderr log
file was named "syslogFile", the C file is named syslogger.c, and there's
an entirely separate syslog (the message logging API) destination. I think
this clears that up a bit.

The patches pass check-world on Linux. I haven't tested it on Windows but
it does pass check-world with EXEC_BACKEND defined to try out the forking
code paths. Definitely needs some review to ensure it's functionally
correct for the different log files.

I haven't tried splitting the csvlog code out or adding the new jsonlog
atop this yet. There's enough changes here that I'd like to get this
settled first.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/

Attachment Content-Type Size
v4-0001-Add-constant-for-list-of-log-destinations-that-use-f.patch text/x-patch 1.8 KB
v4-0002-Split-out-syslogger-EXEC_BACKEND-fd-serialization-an.patch text/x-patch 3.8 KB
v4-0003-Refactor-syslogger-to-consolidate-common-tasks-for-f.patch text/x-patch 17.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2021-09-16 22:58:25 Re: Possible fault with resolve column name (plpgsql)
Previous Message Bossart, Nathan 2021-09-16 21:26:56 Re: Estimating HugePages Requirements?