Refactoring syslogger piping to simplify adding new log destinations

From: Sehrope Sarkuni <sehrope(at)jackdb(dot)com>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Refactoring syslogger piping to simplify adding new log destinations
Date: 2019-07-10 21:05:09
Message-ID: CAH7T-apGDyRMA5MJad80JbWa+2zwbPFLJ_jfJdFjCO=YV6g14Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

While working on adding a new log_destination I noticed that the
syslogger piping would need to be updated. At the moment both ends
only handle stderr/csvlog as the pipe message header has a char
"is_last" that is either t/f (stderr last, stderr partial) or T/F
(csvlog last, csvlog partial). Couple approaches came to mind:

1. Use additional pairs of chars for each additional destination (e.g.
x/X, y/Y, ...) and mimic the logic of csvlog.
2. Repurpose the char "is_last" as a bitmap of the log destination
with the highest order bit indicating whether it's the last chunk.
3. Add a separate field "dest" for the log destination and leave
"is_last" as a t/f indicating whether it's the last chunk.

Attached are patches for each approach (fun exercise!). Also attached
is a basic TAP test to invoke the csvlog destination. It's a clone of
pg_ctl log rotation test that looks for .csv logs. If there's interest
in the test I was thinking of expanding it a bit to include "big"
output that would span multiple messages to test the partial/combining
path. My thoughts on the approaches:

#1 doesn't change the header types or size but seems ugly as it leads
to new pairs of constants and logic in multiple places. In particular,
both send and receive ends have to encode and decode the destination.
#2 is cleaner as there's a logical separation of the dest fields and
no need for new constant pairs when adding new destinations. Would
also need to ensure new LOG_DESTINATION_xyz constants do not use that
last bit (there's already four now so room for three more).
#3 leads to the cleanest code though you lose 4-bytes of max data size
per chunk.

Which would be preferable? I'd like to validate the approach as the
new log destination would be built atop it. I leaning toward #3 though
if the 4-byte loss is a deal breaker then #2.

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

Attachment Content-Type Size
0001-Add-basic-test-for-csvlog.patch text/x-patch 2.8 KB
0002-Option-1-Refactor-syslogger-piping-to-use-constants.patch text/x-patch 4.3 KB
0002-Option-2-Bitmap-to-combine-dest-in-pipe-header.patch text/x-patch 4.6 KB
0002-Option-3-Add-separate-dest-field-to-header.patch text/x-patch 4.5 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2019-07-10 21:26:09 Re: [sqlsmith] Crash in mcv_get_match_bitmap
Previous Message Tom Lane 2019-07-10 20:57:54 Re: [sqlsmith] Crash in mcv_get_match_bitmap