Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commitbff84a5

Browse files
committed
Make syslogger more robust against failures in opening CSV log files.
The previous coding figured it'd be good enough to postpone openingthe first CSV log file until we got a message we needed to write there.This is unsafe, though, because if the open fails we end up in infiniterecursion trying to report the failure. Instead make the CSV log filemanagement code look as nearly as possible like the longstanding logicfor the stderr log file. In particular, open it immediately at postmasterstartup (if enabled), or when we get a SIGHUP in which we find thatlog_destination has been changed to enable CSV logging.It seems OK to fail if a postmaster-start-time open attempt fails, aswe've long done for the stderr log file. But we can't die if we failto open a CSV log file during SIGHUP, so we're still left with a problem.In that case, write any output meant for the CSV log file to the stderrlog file. (This will also cover race-condition cases in which backendssend CSV log data before or after we have the CSV log file open.)This patch also fixes an ancient oversight that, if CSV logging wasturned off during a SIGHUP, we never actually closed the last CSVlog file.In passing, remember to reset whereToSendOutput = DestNone during sysloggerstart, since (unlike all other postmaster children) it's forked before thepostmaster has done that. This made for a platform-dependent differencein error reporting behavior between the syslogger and other children:except on Windows, it'd report problems to the original postmaster stderras well as the normal error log file(s). It's barely possible that thatwas intentional at some point; but it doesn't seem likely to be desirablein production, and the platform dependency definitely isn't desirable.Per report from Alexander Kukushkin. It's been like this for a long time,so back-patch to all supported branches.Discussion:https://postgr.es/m/CAFh8B==iLUD_gqC-dAENS0V+kVrCeGiKujtKqSQ7++S-caaChw@mail.gmail.com
1 parentba9d35b commitbff84a5

File tree

1 file changed

+123
-39
lines changed

1 file changed

+123
-39
lines changed

‎src/backend/postmaster/syslogger.c‎

Lines changed: 123 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#include"storage/ipc.h"
4646
#include"storage/latch.h"
4747
#include"storage/pg_shmem.h"
48+
#include"tcop/tcopprot.h"
4849
#include"utils/guc.h"
4950
#include"utils/ps_status.h"
5051
#include"utils/timestamp.h"
@@ -135,7 +136,6 @@ static void syslogger_parseArgs(int argc, char *argv[]);
135136
NON_EXEC_STATICvoidSysLoggerMain(intargc,char*argv[])pg_attribute_noreturn();
136137
staticvoidprocess_pipe_input(char*logbuffer,int*bytes_in_logbuffer);
137138
staticvoidflush_pipe_input(char*logbuffer,int*bytes_in_logbuffer);
138-
staticvoidopen_csvlogfile(void);
139139
staticFILE*logfile_open(constchar*filename,constchar*mode,
140140
boolallow_errors);
141141

@@ -272,11 +272,13 @@ SysLoggerMain(int argc, char *argv[])
272272
#endif/* WIN32 */
273273

274274
/*
275-
* Remember activelogfile's name. We recomputethis from the reference
275+
* Remember activelogfiles' name(s). We recompute'em from the reference
276276
* time because passing down just the pg_time_t is a lot cheaper than
277277
* passing a whole file path in the EXEC_BACKEND case.
278278
*/
279279
last_file_name=logfile_getname(first_syslogger_file_time,NULL);
280+
if (csvlogFile!=NULL)
281+
last_csv_file_name=logfile_getname(first_syslogger_file_time,".csv");
280282

281283
/* remember active logfile parameters */
282284
currentLogDir=pstrdup(Log_directory);
@@ -286,6 +288,13 @@ SysLoggerMain(int argc, char *argv[])
286288
set_next_rotation_time();
287289
update_metainfo_datafile();
288290

291+
/*
292+
* Reset whereToSendOutput, as the postmaster will do (but hasn't yet, at
293+
* the point where we forked). This prevents duplicate output of messages
294+
* from syslogger itself.
295+
*/
296+
whereToSendOutput=DestNone;
297+
289298
/* main worker loop */
290299
for (;;)
291300
{
@@ -332,6 +341,14 @@ SysLoggerMain(int argc, char *argv[])
332341
rotation_requested= true;
333342
}
334343

344+
/*
345+
* Force a rotation if CSVLOG output was just turned on or off and
346+
* we need to open or close csvlogFile accordingly.
347+
*/
348+
if (((Log_destination&LOG_DESTINATION_CSVLOG)!=0)!=
349+
(csvlogFile!=NULL))
350+
rotation_requested= true;
351+
335352
/*
336353
* If rotation time parameter changed, reset next rotation time,
337354
* but don't immediately force a rotation.
@@ -580,12 +597,27 @@ SysLogger_Start(void)
580597
* a time-based rotation.
581598
*/
582599
first_syslogger_file_time=time(NULL);
600+
583601
filename=logfile_getname(first_syslogger_file_time,NULL);
584602

585603
syslogFile=logfile_open(filename,"a", false);
586604

587605
pfree(filename);
588606

607+
/*
608+
* Likewise for the initial CSV log file, if that's enabled. (Note that
609+
* we open syslogFile even when only CSV output is nominally enabled,
610+
* since some code paths will write to syslogFile anyway.)
611+
*/
612+
if (Log_destination&LOG_DESTINATION_CSVLOG)
613+
{
614+
filename=logfile_getname(first_syslogger_file_time,".csv");
615+
616+
csvlogFile=logfile_open(filename,"a", false);
617+
618+
pfree(filename);
619+
}
620+
589621
#ifdefEXEC_BACKEND
590622
switch ((sysloggerPid=syslogger_forkexec()))
591623
#else
@@ -675,9 +707,14 @@ SysLogger_Start(void)
675707
redirection_done= true;
676708
}
677709

678-
/* postmaster will never write the file; closeit */
710+
/* postmaster will never write the file(s); close'em */
679711
fclose(syslogFile);
680712
syslogFile=NULL;
713+
if (csvlogFile!=NULL)
714+
{
715+
fclose(csvlogFile);
716+
csvlogFile=NULL;
717+
}
681718
return (int)sysloggerPid;
682719
}
683720

@@ -699,6 +736,7 @@ syslogger_forkexec(void)
699736
char*av[10];
700737
intac=0;
701738
charfilenobuf[32];
739+
charcsvfilenobuf[32];
702740

703741
av[ac++]="postgres";
704742
av[ac++]="--forklog";
@@ -720,6 +758,21 @@ syslogger_forkexec(void)
720758
#endif/* WIN32 */
721759
av[ac++]=filenobuf;
722760

761+
#ifndefWIN32
762+
if (csvlogFile!=NULL)
763+
snprintf(csvfilenobuf,sizeof(csvfilenobuf),"%d",
764+
fileno(csvlogFile));
765+
else
766+
strcpy(csvfilenobuf,"-1");
767+
#else/* WIN32 */
768+
if (csvlogFile!=NULL)
769+
snprintf(csvfilenobuf,sizeof(csvfilenobuf),"%ld",
770+
(long)_get_osfhandle(_fileno(csvlogFile)));
771+
else
772+
strcpy(csvfilenobuf,"0");
773+
#endif/* WIN32 */
774+
av[ac++]=csvfilenobuf;
775+
723776
av[ac]=NULL;
724777
Assert(ac<lengthof(av));
725778

@@ -736,16 +789,29 @@ syslogger_parseArgs(int argc, char *argv[])
736789
{
737790
intfd;
738791

739-
Assert(argc==4);
792+
Assert(argc==5);
740793
argv+=3;
741794

795+
/*
796+
* Re-open the error output files that were opened by SysLogger_Start().
797+
*
798+
* We expect this will always succeed, which is too optimistic, but if it
799+
* fails there's not a lot we can do to report the problem anyway. As
800+
* coded, we'll just crash on a null pointer dereference after failure...
801+
*/
742802
#ifndefWIN32
743803
fd=atoi(*argv++);
744804
if (fd!=-1)
745805
{
746806
syslogFile=fdopen(fd,"a");
747807
setvbuf(syslogFile,NULL,PG_IOLBF,0);
748808
}
809+
fd=atoi(*argv++);
810+
if (fd!=-1)
811+
{
812+
csvlogFile=fdopen(fd,"a");
813+
setvbuf(csvlogFile,NULL,PG_IOLBF,0);
814+
}
749815
#else/* WIN32 */
750816
fd=atoi(*argv++);
751817
if (fd!=0)
@@ -757,6 +823,16 @@ syslogger_parseArgs(int argc, char *argv[])
757823
setvbuf(syslogFile,NULL,PG_IOLBF,0);
758824
}
759825
}
826+
fd=atoi(*argv++);
827+
if (fd!=0)
828+
{
829+
fd=_open_osfhandle(fd,_O_APPEND |_O_TEXT);
830+
if (fd>0)
831+
{
832+
csvlogFile=fdopen(fd,"a");
833+
setvbuf(csvlogFile,NULL,PG_IOLBF,0);
834+
}
835+
}
760836
#endif/* WIN32 */
761837
}
762838
#endif/* EXEC_BACKEND */
@@ -998,13 +1074,29 @@ write_syslogger_file(const char *buffer, int count, int destination)
9981074
intrc;
9991075
FILE*logfile;
10001076

1001-
if (destination==LOG_DESTINATION_CSVLOG&&csvlogFile==NULL)
1002-
open_csvlogfile();
1077+
/*
1078+
* If we're told to write to csvlogFile, but it's not open, dump the data
1079+
* to syslogFile (which is always open) instead. This can happen if CSV
1080+
* output is enabled after postmaster start and we've been unable to open
1081+
* csvlogFile. There are also race conditions during a parameter change
1082+
* whereby backends might send us CSV output before we open csvlogFile or
1083+
* after we close it. Writing CSV-formatted output to the regular log
1084+
* file isn't great, but it beats dropping log output on the floor.
1085+
*
1086+
* Think not to improve this by trying to open csvlogFile on-the-fly. Any
1087+
* failure in that would lead to recursion.
1088+
*/
1089+
logfile= (destination==LOG_DESTINATION_CSVLOG&&
1090+
csvlogFile!=NULL) ?csvlogFile :syslogFile;
10031091

1004-
logfile=destination==LOG_DESTINATION_CSVLOG ?csvlogFile :syslogFile;
10051092
rc=fwrite(buffer,1,count,logfile);
10061093

1007-
/* can't use ereport here because of possible recursion */
1094+
/*
1095+
* Try to report any failure. We mustn't use ereport because it would
1096+
* just recurse right back here, but write_stderr is OK: it will write
1097+
* either to the postmaster's original stderr, or to /dev/null, but never
1098+
* to our input pipe which would result in a different sort of looping.
1099+
*/
10081100
if (rc!=count)
10091101
write_stderr("could not write to log file: %s\n",strerror(errno));
10101102
}
@@ -1087,31 +1179,6 @@ pipeThread(void *arg)
10871179
}
10881180
#endif/* WIN32 */
10891181

1090-
/*
1091-
* Open the csv log file - we do this opportunistically, because
1092-
* we don't know if CSV logging will be wanted.
1093-
*
1094-
* This is only used the first time we open the csv log in a given syslogger
1095-
* process, not during rotations. As with opening the main log file, we
1096-
* always append in this situation.
1097-
*/
1098-
staticvoid
1099-
open_csvlogfile(void)
1100-
{
1101-
char*filename;
1102-
1103-
filename=logfile_getname(time(NULL),".csv");
1104-
1105-
csvlogFile=logfile_open(filename,"a", false);
1106-
1107-
if (last_csv_file_name!=NULL)/* probably shouldn't happen */
1108-
pfree(last_csv_file_name);
1109-
1110-
last_csv_file_name=filename;
1111-
1112-
update_metainfo_datafile();
1113-
}
1114-
11151182
/*
11161183
* Open a new logfile with proper permissions and buffering options.
11171184
*
@@ -1179,7 +1246,7 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
11791246
else
11801247
fntime=time(NULL);
11811248
filename=logfile_getname(fntime,NULL);
1182-
if (csvlogFile!=NULL)
1249+
if (Log_destination&LOG_DESTINATION_CSVLOG)
11831250
csvfilename=logfile_getname(fntime,".csv");
11841251

11851252
/*
@@ -1231,10 +1298,16 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
12311298
filename=NULL;
12321299
}
12331300

1234-
/* Same as above, but for csv file. */
1235-
1236-
if (csvlogFile!=NULL&&
1237-
(time_based_rotation|| (size_rotation_for&LOG_DESTINATION_CSVLOG)))
1301+
/*
1302+
* Same as above, but for csv file. Note that if LOG_DESTINATION_CSVLOG
1303+
* was just turned on, we might have to open csvlogFile here though it was
1304+
* not open before. In such a case we'll append not overwrite (since
1305+
* last_csv_file_name will be NULL); that is consistent with the normal
1306+
* rules since it's not a time-based rotation.
1307+
*/
1308+
if ((Log_destination&LOG_DESTINATION_CSVLOG)&&
1309+
(csvlogFile==NULL||
1310+
time_based_rotation|| (size_rotation_for&LOG_DESTINATION_CSVLOG)))
12381311
{
12391312
if (Log_truncate_on_rotation&&time_based_rotation&&
12401313
last_csv_file_name!=NULL&&
@@ -1265,7 +1338,8 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
12651338
return;
12661339
}
12671340

1268-
fclose(csvlogFile);
1341+
if (csvlogFile!=NULL)
1342+
fclose(csvlogFile);
12691343
csvlogFile=fh;
12701344

12711345
/* instead of pfree'ing filename, remember it for next time */
@@ -1274,6 +1348,16 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
12741348
last_csv_file_name=csvfilename;
12751349
csvfilename=NULL;
12761350
}
1351+
elseif (!(Log_destination&LOG_DESTINATION_CSVLOG)&&
1352+
csvlogFile!=NULL)
1353+
{
1354+
/* CSVLOG was just turned off, so close the old file */
1355+
fclose(csvlogFile);
1356+
csvlogFile=NULL;
1357+
if (last_csv_file_name!=NULL)
1358+
pfree(last_csv_file_name);
1359+
last_csv_file_name=NULL;
1360+
}
12771361

12781362
if (filename)
12791363
pfree(filename);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp