Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 2d77d83

Browse filesBrowse files
michaelpqmichaelpq
committed
Refactor the syslogger pipe protocol to use a bitmask for its options
The previous protocol expected a set of matching characters to check if a message sent was the last one or not, that changed depending on the destination wanted: - 't' and 'f' tracked the last message of a log sent to stderr. - 'T' and 'F' tracked the last message of a log sent to csvlog. This could be extended with more characters when introducing new destinations, but using a bitmask is much more elegant. This commit changes the protocol so as a bitmask is used in the header of a log chunk message sent to the syslogger, with the following options available for now: - log_destination as stderr. - log_destination as csvlog. - if a message is the last chunk of a message. Sehrope found this issue in a patch set to introduce JSON as an option for log_destination, but his patch made the size of the protocol header larger. This commit keeps the same size as the original, and adapts the protocol as wanted. Thanks also to Andrew Dunstan and Greg Stark for the discussion. Author: Michael Paquier, Sehrope Sarkuni Discussion: https://postgr.es/m/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com
1 parent e757080 commit 2d77d83
Copy full SHA for 2d77d83

File tree

Expand file treeCollapse file tree

3 files changed

+27
-9
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+27
-9
lines changed

‎src/backend/postmaster/syslogger.c

Copy file name to clipboardExpand all lines: src/backend/postmaster/syslogger.c
+14-5Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
#include "nodes/pg_list.h"
3939
#include "pgstat.h"
4040
#include "pgtime.h"
41+
#include "port/pg_bitutils.h"
4142
#include "postmaster/fork_process.h"
4243
#include "postmaster/interrupt.h"
4344
#include "postmaster/postmaster.h"
@@ -885,14 +886,15 @@ process_pipe_input(char *logbuffer, int *bytes_in_logbuffer)
885886
{
886887
PipeProtoHeader p;
887888
int chunklen;
889+
bits8 dest_flags;
888890

889891
/* Do we have a valid header? */
890892
memcpy(&p, cursor, offsetof(PipeProtoHeader, data));
893+
dest_flags = p.flags & (PIPE_PROTO_DEST_STDERR | PIPE_PROTO_DEST_CSVLOG);
891894
if (p.nuls[0] == '\0' && p.nuls[1] == '\0' &&
892895
p.len > 0 && p.len <= PIPE_MAX_PAYLOAD &&
893896
p.pid != 0 &&
894-
(p.is_last == 't' || p.is_last == 'f' ||
895-
p.is_last == 'T' || p.is_last == 'F'))
897+
pg_popcount((char *) &dest_flags, 1) == 1)
896898
{
897899
List *buffer_list;
898900
ListCell *cell;
@@ -906,8 +908,15 @@ process_pipe_input(char *logbuffer, int *bytes_in_logbuffer)
906908
if (count < chunklen)
907909
break;
908910

909-
dest = (p.is_last == 'T' || p.is_last == 'F') ?
910-
LOG_DESTINATION_CSVLOG : LOG_DESTINATION_STDERR;
911+
if ((p.flags & PIPE_PROTO_DEST_STDERR) != 0)
912+
dest = LOG_DESTINATION_STDERR;
913+
else if ((p.flags & PIPE_PROTO_DEST_CSVLOG) != 0)
914+
dest = LOG_DESTINATION_CSVLOG;
915+
else
916+
{
917+
/* this should never happen as of the header validation */
918+
Assert(false);
919+
}
911920

912921
/* Locate any existing buffer for this source pid */
913922
buffer_list = buffer_lists[p.pid % NBUFFER_LISTS];
@@ -924,7 +933,7 @@ process_pipe_input(char *logbuffer, int *bytes_in_logbuffer)
924933
free_slot = buf;
925934
}
926935

927-
if (p.is_last == 'f' || p.is_last == 'F')
936+
if ((p.flags & PIPE_PROTO_IS_LAST) == 0)
928937
{
929938
/*
930939
* Save a complete non-final chunk in a per-pid buffer

‎src/backend/utils/error/elog.c

Copy file name to clipboardExpand all lines: src/backend/utils/error/elog.c
+7-2Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3250,11 +3250,16 @@ write_pipe_chunks(char *data, int len, int dest)
32503250

32513251
p.proto.nuls[0] = p.proto.nuls[1] = '\0';
32523252
p.proto.pid = MyProcPid;
3253+
p.proto.flags = 0;
3254+
if (dest == LOG_DESTINATION_STDERR)
3255+
p.proto.flags |= PIPE_PROTO_DEST_STDERR;
3256+
else if (dest == LOG_DESTINATION_CSVLOG)
3257+
p.proto.flags |= PIPE_PROTO_DEST_CSVLOG;
32533258

32543259
/* write all but the last chunk */
32553260
while (len > PIPE_MAX_PAYLOAD)
32563261
{
3257-
p.proto.is_last = (dest == LOG_DESTINATION_CSVLOG ? 'F' : 'f');
3262+
/* no need to set PIPE_PROTO_IS_LAST yet */
32583263
p.proto.len = PIPE_MAX_PAYLOAD;
32593264
memcpy(p.proto.data, data, PIPE_MAX_PAYLOAD);
32603265
rc = write(fd, &p, PIPE_HEADER_SIZE + PIPE_MAX_PAYLOAD);
@@ -3264,7 +3269,7 @@ write_pipe_chunks(char *data, int len, int dest)
32643269
}
32653270

32663271
/* write the last chunk */
3267-
p.proto.is_last = (dest == LOG_DESTINATION_CSVLOG ? 'T' : 't');
3272+
p.proto.flags |= PIPE_PROTO_IS_LAST;
32683273
p.proto.len = len;
32693274
memcpy(p.proto.data, data, len);
32703275
rc = write(fd, &p, PIPE_HEADER_SIZE + len);

‎src/include/postmaster/syslogger.h

Copy file name to clipboardExpand all lines: src/include/postmaster/syslogger.h
+6-2Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ typedef struct
4646
char nuls[2]; /* always \0\0 */
4747
uint16 len; /* size of this chunk (counts data only) */
4848
int32 pid; /* writer's pid */
49-
char is_last; /* last chunk of message? 't' or 'f' ('T' or
50-
* 'F' for CSV case) */
49+
bits8 flags; /* bitmask of PIPE_PROTO_* */
5150
char data[FLEXIBLE_ARRAY_MEMBER]; /* data payload starts here */
5251
} PipeProtoHeader;
5352

@@ -60,6 +59,11 @@ typedef union
6059
#define PIPE_HEADER_SIZE offsetof(PipeProtoHeader, data)
6160
#define PIPE_MAX_PAYLOAD ((int) (PIPE_CHUNK_SIZE - PIPE_HEADER_SIZE))
6261

62+
/* flag bits for PipeProtoHeader->flags */
63+
#define PIPE_PROTO_IS_LAST 0x01 /* last chunk of message? */
64+
/* log destinations */
65+
#define PIPE_PROTO_DEST_STDERR 0x10
66+
#define PIPE_PROTO_DEST_CSVLOG 0x20
6367

6468
/* GUC options */
6569
extern bool Logging_collector;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.