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

Commit93ca07f

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 parentc2ea62f commit93ca07f

File tree

1 file changed

+123
-37
lines changed

1 file changed

+123
-37
lines changed

‎src/backend/postmaster/syslogger.c‎

Lines changed: 123 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include"storage/ipc.h"
4444
#include"storage/latch.h"
4545
#include"storage/pg_shmem.h"
46+
#include"tcop/tcopprot.h"
4647
#include"utils/guc.h"
4748
#include"utils/ps_status.h"
4849
#include"utils/timestamp.h"
@@ -133,7 +134,6 @@ static void syslogger_parseArgs(int argc, char *argv[]);
133134
NON_EXEC_STATICvoidSysLoggerMain(intargc,char*argv[])pg_attribute_noreturn();
134135
staticvoidprocess_pipe_input(char*logbuffer,int*bytes_in_logbuffer);
135136
staticvoidflush_pipe_input(char*logbuffer,int*bytes_in_logbuffer);
136-
staticvoidopen_csvlogfile(void);
137137
staticFILE*logfile_open(constchar*filename,constchar*mode,
138138
boolallow_errors);
139139

@@ -269,11 +269,13 @@ SysLoggerMain(int argc, char *argv[])
269269
#endif/* WIN32 */
270270

271271
/*
272-
* Remember activelogfile's name. We recomputethis from the reference
272+
* Remember activelogfiles' name(s). We recompute'em from the reference
273273
* time because passing down just the pg_time_t is a lot cheaper than
274274
* passing a whole file path in the EXEC_BACKEND case.
275275
*/
276276
last_file_name=logfile_getname(first_syslogger_file_time,NULL);
277+
if (csvlogFile!=NULL)
278+
last_csv_file_name=logfile_getname(first_syslogger_file_time,".csv");
277279

278280
/* remember active logfile parameters */
279281
currentLogDir=pstrdup(Log_directory);
@@ -282,6 +284,13 @@ SysLoggerMain(int argc, char *argv[])
282284
/* set next planned rotation time */
283285
set_next_rotation_time();
284286

287+
/*
288+
* Reset whereToSendOutput, as the postmaster will do (but hasn't yet, at
289+
* the point where we forked). This prevents duplicate output of messages
290+
* from syslogger itself.
291+
*/
292+
whereToSendOutput=DestNone;
293+
285294
/* main worker loop */
286295
for (;;)
287296
{
@@ -328,6 +337,14 @@ SysLoggerMain(int argc, char *argv[])
328337
rotation_requested= true;
329338
}
330339

340+
/*
341+
* Force a rotation if CSVLOG output was just turned on or off and
342+
* we need to open or close csvlogFile accordingly.
343+
*/
344+
if (((Log_destination&LOG_DESTINATION_CSVLOG)!=0)!=
345+
(csvlogFile!=NULL))
346+
rotation_requested= true;
347+
331348
/*
332349
* If rotation time parameter changed, reset next rotation time,
333350
* but don't immediately force a rotation.
@@ -567,12 +584,27 @@ SysLogger_Start(void)
567584
* a time-based rotation.
568585
*/
569586
first_syslogger_file_time=time(NULL);
587+
570588
filename=logfile_getname(first_syslogger_file_time,NULL);
571589

572590
syslogFile=logfile_open(filename,"a", false);
573591

574592
pfree(filename);
575593

594+
/*
595+
* Likewise for the initial CSV log file, if that's enabled. (Note that
596+
* we open syslogFile even when only CSV output is nominally enabled,
597+
* since some code paths will write to syslogFile anyway.)
598+
*/
599+
if (Log_destination&LOG_DESTINATION_CSVLOG)
600+
{
601+
filename=logfile_getname(first_syslogger_file_time,".csv");
602+
603+
csvlogFile=logfile_open(filename,"a", false);
604+
605+
pfree(filename);
606+
}
607+
576608
#ifdefEXEC_BACKEND
577609
switch ((sysloggerPid=syslogger_forkexec()))
578610
#else
@@ -662,9 +694,14 @@ SysLogger_Start(void)
662694
redirection_done= true;
663695
}
664696

665-
/* postmaster will never write the file; closeit */
697+
/* postmaster will never write the file(s); close'em */
666698
fclose(syslogFile);
667699
syslogFile=NULL;
700+
if (csvlogFile!=NULL)
701+
{
702+
fclose(csvlogFile);
703+
csvlogFile=NULL;
704+
}
668705
return (int)sysloggerPid;
669706
}
670707

@@ -686,6 +723,7 @@ syslogger_forkexec(void)
686723
char*av[10];
687724
intac=0;
688725
charfilenobuf[32];
726+
charcsvfilenobuf[32];
689727

690728
av[ac++]="postgres";
691729
av[ac++]="--forklog";
@@ -707,6 +745,21 @@ syslogger_forkexec(void)
707745
#endif/* WIN32 */
708746
av[ac++]=filenobuf;
709747

748+
#ifndefWIN32
749+
if (csvlogFile!=NULL)
750+
snprintf(csvfilenobuf,sizeof(csvfilenobuf),"%d",
751+
fileno(csvlogFile));
752+
else
753+
strcpy(csvfilenobuf,"-1");
754+
#else/* WIN32 */
755+
if (csvlogFile!=NULL)
756+
snprintf(csvfilenobuf,sizeof(csvfilenobuf),"%ld",
757+
(long)_get_osfhandle(_fileno(csvlogFile)));
758+
else
759+
strcpy(csvfilenobuf,"0");
760+
#endif/* WIN32 */
761+
av[ac++]=csvfilenobuf;
762+
710763
av[ac]=NULL;
711764
Assert(ac<lengthof(av));
712765

@@ -723,16 +776,29 @@ syslogger_parseArgs(int argc, char *argv[])
723776
{
724777
intfd;
725778

726-
Assert(argc==4);
779+
Assert(argc==5);
727780
argv+=3;
728781

782+
/*
783+
* Re-open the error output files that were opened by SysLogger_Start().
784+
*
785+
* We expect this will always succeed, which is too optimistic, but if it
786+
* fails there's not a lot we can do to report the problem anyway. As
787+
* coded, we'll just crash on a null pointer dereference after failure...
788+
*/
729789
#ifndefWIN32
730790
fd=atoi(*argv++);
731791
if (fd!=-1)
732792
{
733793
syslogFile=fdopen(fd,"a");
734794
setvbuf(syslogFile,NULL,PG_IOLBF,0);
735795
}
796+
fd=atoi(*argv++);
797+
if (fd!=-1)
798+
{
799+
csvlogFile=fdopen(fd,"a");
800+
setvbuf(csvlogFile,NULL,PG_IOLBF,0);
801+
}
736802
#else/* WIN32 */
737803
fd=atoi(*argv++);
738804
if (fd!=0)
@@ -744,6 +810,16 @@ syslogger_parseArgs(int argc, char *argv[])
744810
setvbuf(syslogFile,NULL,PG_IOLBF,0);
745811
}
746812
}
813+
fd=atoi(*argv++);
814+
if (fd!=0)
815+
{
816+
fd=_open_osfhandle(fd,_O_APPEND |_O_TEXT);
817+
if (fd>0)
818+
{
819+
csvlogFile=fdopen(fd,"a");
820+
setvbuf(csvlogFile,NULL,PG_IOLBF,0);
821+
}
822+
}
747823
#endif/* WIN32 */
748824
}
749825
#endif/* EXEC_BACKEND */
@@ -985,13 +1061,29 @@ write_syslogger_file(const char *buffer, int count, int destination)
9851061
intrc;
9861062
FILE*logfile;
9871063

988-
if (destination==LOG_DESTINATION_CSVLOG&&csvlogFile==NULL)
989-
open_csvlogfile();
1064+
/*
1065+
* If we're told to write to csvlogFile, but it's not open, dump the data
1066+
* to syslogFile (which is always open) instead. This can happen if CSV
1067+
* output is enabled after postmaster start and we've been unable to open
1068+
* csvlogFile. There are also race conditions during a parameter change
1069+
* whereby backends might send us CSV output before we open csvlogFile or
1070+
* after we close it. Writing CSV-formatted output to the regular log
1071+
* file isn't great, but it beats dropping log output on the floor.
1072+
*
1073+
* Think not to improve this by trying to open csvlogFile on-the-fly. Any
1074+
* failure in that would lead to recursion.
1075+
*/
1076+
logfile= (destination==LOG_DESTINATION_CSVLOG&&
1077+
csvlogFile!=NULL) ?csvlogFile :syslogFile;
9901078

991-
logfile=destination==LOG_DESTINATION_CSVLOG ?csvlogFile :syslogFile;
9921079
rc=fwrite(buffer,1,count,logfile);
9931080

994-
/* can't use ereport here because of possible recursion */
1081+
/*
1082+
* Try to report any failure. We mustn't use ereport because it would
1083+
* just recurse right back here, but write_stderr is OK: it will write
1084+
* either to the postmaster's original stderr, or to /dev/null, but never
1085+
* to our input pipe which would result in a different sort of looping.
1086+
*/
9951087
if (rc!=count)
9961088
write_stderr("could not write to log file: %s\n",strerror(errno));
9971089
}
@@ -1074,29 +1166,6 @@ pipeThread(void *arg)
10741166
}
10751167
#endif/* WIN32 */
10761168

1077-
/*
1078-
* Open the csv log file - we do this opportunistically, because
1079-
* we don't know if CSV logging will be wanted.
1080-
*
1081-
* This is only used the first time we open the csv log in a given syslogger
1082-
* process, not during rotations. As with opening the main log file, we
1083-
* always append in this situation.
1084-
*/
1085-
staticvoid
1086-
open_csvlogfile(void)
1087-
{
1088-
char*filename;
1089-
1090-
filename=logfile_getname(time(NULL),".csv");
1091-
1092-
csvlogFile=logfile_open(filename,"a", false);
1093-
1094-
if (last_csv_file_name!=NULL)/* probably shouldn't happen */
1095-
pfree(last_csv_file_name);
1096-
1097-
last_csv_file_name=filename;
1098-
}
1099-
11001169
/*
11011170
* Open a new logfile with proper permissions and buffering options.
11021171
*
@@ -1164,7 +1233,7 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
11641233
else
11651234
fntime=time(NULL);
11661235
filename=logfile_getname(fntime,NULL);
1167-
if (csvlogFile!=NULL)
1236+
if (Log_destination&LOG_DESTINATION_CSVLOG)
11681237
csvfilename=logfile_getname(fntime,".csv");
11691238

11701239
/*
@@ -1216,10 +1285,16 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
12161285
filename=NULL;
12171286
}
12181287

1219-
/* Same as above, but for csv file. */
1220-
1221-
if (csvlogFile!=NULL&&
1222-
(time_based_rotation|| (size_rotation_for&LOG_DESTINATION_CSVLOG)))
1288+
/*
1289+
* Same as above, but for csv file. Note that if LOG_DESTINATION_CSVLOG
1290+
* was just turned on, we might have to open csvlogFile here though it was
1291+
* not open before. In such a case we'll append not overwrite (since
1292+
* last_csv_file_name will be NULL); that is consistent with the normal
1293+
* rules since it's not a time-based rotation.
1294+
*/
1295+
if ((Log_destination&LOG_DESTINATION_CSVLOG)&&
1296+
(csvlogFile==NULL||
1297+
time_based_rotation|| (size_rotation_for&LOG_DESTINATION_CSVLOG)))
12231298
{
12241299
if (Log_truncate_on_rotation&&time_based_rotation&&
12251300
last_csv_file_name!=NULL&&
@@ -1250,7 +1325,8 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
12501325
return;
12511326
}
12521327

1253-
fclose(csvlogFile);
1328+
if (csvlogFile!=NULL)
1329+
fclose(csvlogFile);
12541330
csvlogFile=fh;
12551331

12561332
/* instead of pfree'ing filename, remember it for next time */
@@ -1259,6 +1335,16 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
12591335
last_csv_file_name=csvfilename;
12601336
csvfilename=NULL;
12611337
}
1338+
elseif (!(Log_destination&LOG_DESTINATION_CSVLOG)&&
1339+
csvlogFile!=NULL)
1340+
{
1341+
/* CSVLOG was just turned off, so close the old file */
1342+
fclose(csvlogFile);
1343+
csvlogFile=NULL;
1344+
if (last_csv_file_name!=NULL)
1345+
pfree(last_csv_file_name);
1346+
last_csv_file_name=NULL;
1347+
}
12621348

12631349
if (filename)
12641350
pfree(filename);

0 commit comments

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