Re: Add jsonlog log_destination for JSON server logs

[Michael Paquier]

On Wed, Sep 08, 2021 at 08:46:44AM -0400, Andrew Dunstan wrote:
> Yeah. A very simple change would be to use two different values for json
> (say 'y' and 'n'). A slightly more principled scheme might use the top
> bit for the end marker and the bottom 3 bits for the dest type (so up to
> 8 types possible), with the rest available for future use.

I was thinking to track stderr as a case where no bits are set in the
flags for the area of the destinations, but that's a bit crazy if we
have a lot of margin in what can be stored.  I have looked at that and
finished with the attached which is an improvement IMO, especially
when it comes to the header validation.

FWIW, while looking at my own external module for the JSON logs, I
noticed that I used the chunk protocol when the log redirection is
enabled, but just enforced everything to be sent to stderr.
--
Michael

diff --git a/src/include/postmaster/syslogger.h 
b/src/include/postmaster/syslogger.h
index 1491eecb0f..c79dfbeba2 100644
--- a/src/include/postmaster/syslogger.h
+++ b/src/include/postmaster/syslogger.h
@@ -46,8 +46,7 @@ typedef struct
        char            nuls[2];                /* always \0\0 */
        uint16          len;                    /* size of this chunk (counts 
data only) */
        int32           pid;                    /* writer's pid */
-       char            is_last;                /* last chunk of message? 't' 
or 'f' ('T' or
-                                                                * 'F' for CSV 
case) */
+       bits8           flags;                  /* bitmask of PIPE_PROTO_* */
        char            data[FLEXIBLE_ARRAY_MEMBER];    /* data payload starts 
here */
 } PipeProtoHeader;
 
@@ -60,6 +59,11 @@ typedef union
 #define PIPE_HEADER_SIZE  offsetof(PipeProtoHeader, data)
 #define PIPE_MAX_PAYLOAD  ((int) (PIPE_CHUNK_SIZE - PIPE_HEADER_SIZE))
 
+/* flag bits for PipeProtoHeader->flags */
+#define PIPE_PROTO_IS_LAST     0x01    /* last chunk of message? */
+/* log destinations */
+#define PIPE_PROTO_DEST_STDERR 0x10
+#define PIPE_PROTO_DEST_CSVLOG 0x20
 
 /* GUC options */
 extern bool Logging_collector;
diff --git a/src/backend/postmaster/syslogger.c 
b/src/backend/postmaster/syslogger.c
index cad43bdef2..c00b2dc186 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -891,8 +891,7 @@ process_pipe_input(char *logbuffer, int *bytes_in_logbuffer)
                if (p.nuls[0] == '\0' && p.nuls[1] == '\0' &&
                        p.len > 0 && p.len <= PIPE_MAX_PAYLOAD &&
                        p.pid != 0 &&
-                       (p.is_last == 't' || p.is_last == 'f' ||
-                        p.is_last == 'T' || p.is_last == 'F'))
+                       (p.flags & (PIPE_PROTO_DEST_STDERR | 
PIPE_PROTO_DEST_CSVLOG)) != 0)
                {
                        List       *buffer_list;
                        ListCell   *cell;
@@ -906,8 +905,15 @@ process_pipe_input(char *logbuffer, int 
*bytes_in_logbuffer)
                        if (count < chunklen)
                                break;
 
-                       dest = (p.is_last == 'T' || p.is_last == 'F') ?
-                               LOG_DESTINATION_CSVLOG : LOG_DESTINATION_STDERR;
+                       if ((p.flags & PIPE_PROTO_DEST_STDERR) != 0)
+                               dest = LOG_DESTINATION_STDERR;
+                       else if ((p.flags & PIPE_PROTO_DEST_CSVLOG) != 0)
+                               dest = LOG_DESTINATION_CSVLOG;
+                       else
+                       {
+                               /* this should never happen as of the header 
validation */
+                               Assert(false);
+                       }
 
                        /* Locate any existing buffer for this source pid */
                        buffer_list = buffer_lists[p.pid % NBUFFER_LISTS];
@@ -924,7 +930,7 @@ process_pipe_input(char *logbuffer, int *bytes_in_logbuffer)
                                        free_slot = buf;
                        }
 
-                       if (p.is_last == 'f' || p.is_last == 'F')
+                       if ((p.flags & PIPE_PROTO_IS_LAST) == 0)
                        {
                                /*
                                 * Save a complete non-final chunk in a per-pid 
buffer
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 816b071afa..2af87ee3bd 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -3250,11 +3250,16 @@ write_pipe_chunks(char *data, int len, int dest)
 
        p.proto.nuls[0] = p.proto.nuls[1] = '\0';
        p.proto.pid = MyProcPid;
+       p.proto.flags = 0;
+       if (dest == LOG_DESTINATION_STDERR)
+               p.proto.flags |= PIPE_PROTO_DEST_STDERR;
+       else if (dest == LOG_DESTINATION_CSVLOG)
+               p.proto.flags |= PIPE_PROTO_DEST_CSVLOG;
 
        /* write all but the last chunk */
        while (len > PIPE_MAX_PAYLOAD)
        {
-               p.proto.is_last = (dest == LOG_DESTINATION_CSVLOG ? 'F' : 'f');
+               /* no need to set PIPE_PROTO_IS_LAST yet */
                p.proto.len = PIPE_MAX_PAYLOAD;
                memcpy(p.proto.data, data, PIPE_MAX_PAYLOAD);
                rc = write(fd, &p, PIPE_HEADER_SIZE + PIPE_MAX_PAYLOAD);
@@ -3264,7 +3269,7 @@ write_pipe_chunks(char *data, int len, int dest)
        }
 
        /* write the last chunk */
-       p.proto.is_last = (dest == LOG_DESTINATION_CSVLOG ? 'T' : 't');
+       p.proto.flags |= PIPE_PROTO_IS_LAST;
        p.proto.len = len;
        memcpy(p.proto.data, data, len);
        rc = write(fd, &p, PIPE_HEADER_SIZE + len);

signature.asc
Description: PGP signature