On Thu, Sep 09, 2021 at 11:17:01AM +0900, Michael Paquier wrote: > 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.
One part that was a bit flacky after more consideration is that the header validation would consider as correct the case where both stderr and csvlog are set in the set of flags. I have finished by just using pg_popcount() on one byte with a filter on the log destinations, making the whole more robust. If there are any objections, please let me know. -- 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..bca3883572 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -38,6 +38,7 @@
#include "nodes/pg_list.h"
#include "pgstat.h"
#include "pgtime.h"
+#include "port/pg_bitutils.h"
#include "postmaster/fork_process.h"
#include "postmaster/interrupt.h"
#include "postmaster/postmaster.h"
@@ -885,14 +886,15 @@ process_pipe_input(char *logbuffer, int
*bytes_in_logbuffer)
{
PipeProtoHeader p;
int chunklen;
+ bits8 dest_flags;
/* Do we have a valid header? */
memcpy(&p, cursor, offsetof(PipeProtoHeader, data));
+ dest_flags = p.flags & (PIPE_PROTO_DEST_STDERR |
PIPE_PROTO_DEST_CSVLOG);
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'))
+ pg_popcount((char *) &dest_flags, 1) == 1)
{
List *buffer_list;
ListCell *cell;
@@ -906,8 +908,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 +933,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
