Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-03-06 Thread Michael Paquier
On Tue, Mar 7, 2017 at 3:21 AM, Robert Haas wrote: > On Sat, Mar 4, 2017 at 10:32 AM, Tom Lane wrote: >> Without having actually looked at this patch, I would say that if it added >> a direct call of fopen() to backend-side code, that was already the

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-03-06 Thread Robert Haas
On Sat, Mar 4, 2017 at 10:32 AM, Tom Lane wrote: > Without having actually looked at this patch, I would say that if it added > a direct call of fopen() to backend-side code, that was already the wrong > thing. Almost always, AllocateFile() would be a better choice, not only

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-03-04 Thread Tom Lane
"Karl O. Pinc" writes: > On Sat, 4 Mar 2017 14:26:54 +0530 > Robert Haas wrote: >> On Fri, Mar 3, 2017 at 7:21 PM, Karl O. Pinc wrote: >>> But if the code does not exit the loop then >>> before calling elog(ERROR), shouldn't it >>> also call

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-03-04 Thread Karl O. Pinc
On Sat, 4 Mar 2017 14:26:54 +0530 Robert Haas wrote: > On Fri, Mar 3, 2017 at 7:21 PM, Karl O. Pinc wrote: > > But if the code does not exit the loop then > > before calling elog(ERROR), shouldn't it > > also call FreeFile(fd)? > > Hmm. Normally error

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-03-04 Thread Robert Haas
On Fri, Mar 3, 2017 at 7:21 PM, Karl O. Pinc wrote: > But if the code does not exit the loop then > before calling elog(ERROR), shouldn't it > also call FreeFile(fd)? Hmm. Normally error recovery cleans up opened files, but since logfile_open() directly calls fopen(), that's not

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-03-04 Thread Robert Haas
On Fri, Mar 3, 2017 at 11:54 AM, Michael Paquier wrote: > On Fri, Mar 3, 2017 at 3:18 PM, Robert Haas wrote: >> Hopefully I haven't broken anything; please let me know if you >> encounter any issues. > > Reading what has just been committed... >

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-03-03 Thread Karl O. Pinc
On Fri, 3 Mar 2017 15:24:53 +0900 Michael Paquier wrote: > > + /* > +* No space found, file content is corrupted. Return > NULL to the > +* caller and inform him on the situation. > +*/ > + elog(ERROR, > +

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-03-02 Thread Michael Paquier
On Fri, Mar 3, 2017 at 3:18 PM, Robert Haas wrote: > Hopefully I haven't broken anything; please let me know if you > encounter any issues. Reading what has just been committed... + /* +* No space found, file content is corrupted. Return NULL to the

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-03-02 Thread Robert Haas
On Mon, Feb 20, 2017 at 4:37 AM, Gilles Darold wrote: >> I think it's sufficient to just remove the file once on postmaster >> startup before trying to launch the syslogger for the first time. >> logging_collector is PGC_POSTMASTER, so if it's not turned on when the >>

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-02-19 Thread Gilles Darold
Le 16/02/2017 à 16:13, Robert Haas a écrit : > On Wed, Feb 15, 2017 at 4:03 PM, Alvaro Herrera > wrote: >> So what is going on here is that SysLogger_Start() wants to unlink the >> current-logfile file if the collector is not enabled. This should >> probably be split

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-02-16 Thread Robert Haas
On Wed, Feb 15, 2017 at 4:03 PM, Alvaro Herrera wrote: > So what is going on here is that SysLogger_Start() wants to unlink the > current-logfile file if the collector is not enabled. This should > probably be split out into a separate new function, for two reasons: >

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-02-15 Thread Tom Lane
"Karl O. Pinc" writes: > On a different topic, I pulled from master and now > I see that git finds the following untracked: > src/bin/pg_basebackup/pg_receivexlog > src/bin/pg_resetxlog/ > src/bin/pg_xlogdump/ Those got renamed, cf

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-02-15 Thread Karl O. Pinc
On Wed, 15 Feb 2017 15:23:00 -0500 Robert Haas wrote: > +ereport(WARNING, > +(errcode(ERRCODE_INTERNAL_ERROR), > + errmsg("corrupted data found in \"%s\"", > +LOG_METAINFO_DATAFILE))); > >

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-02-15 Thread Tom Lane
"Karl O. Pinc" writes: > On Wed, 15 Feb 2017 15:23:00 -0500 > Robert Haas wrote: >> I think our preferred style is not to break strings across lines like >> this. > How do you do that and not exceed the 80 character line limit? > Just ignore the line length

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-02-15 Thread Alvaro Herrera
Robert Haas wrote: > On Fri, Jan 20, 2017 at 2:09 AM, Michael Paquier > wrote: > > Okay I just did it. At the same time the check for ferror is not > > necessary as fgets() returns NULL on an error as well so that's dead > > code. I have also removed the useless call to

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-02-15 Thread Karl O. Pinc
On Wed, 15 Feb 2017 15:23:00 -0500 Robert Haas wrote: > + errhint("The supported log formats are > \"stderr\"" > +" and \"csvlog\"."))); > > I think our preferred style is not to break strings across lines like >

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-02-15 Thread Robert Haas
On Fri, Jan 20, 2017 at 2:09 AM, Michael Paquier wrote: > Okay I just did it. At the same time the check for ferror is not > necessary as fgets() returns NULL on an error as well so that's dead > code. I have also removed the useless call to FreeFile(). diff --git

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-30 Thread Michael Paquier
On Fri, Jan 20, 2017 at 10:14 PM, Michael Paquier wrote: > On Fri, Jan 20, 2017 at 10:11 PM, Alvaro Herrera > wrote: >>> diff --git a/src/backend/replication/basebackup.c >>> b/src/backend/replication/basebackup.c >> >>> @@ -148,6 +149,9 @@

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-20 Thread Michael Paquier
On Fri, Jan 20, 2017 at 10:11 PM, Alvaro Herrera wrote: >> diff --git a/src/backend/replication/basebackup.c >> b/src/backend/replication/basebackup.c > >> @@ -148,6 +149,9 @@ static const char *excludeFiles[] = >> /* Skip auto conf temporary file. */ >>

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-20 Thread Alvaro Herrera
> diff --git a/src/backend/replication/basebackup.c > b/src/backend/replication/basebackup.c > @@ -148,6 +149,9 @@ static const char *excludeFiles[] = > /* Skip auto conf temporary file. */ > PG_AUTOCONF_FILENAME ".tmp", > > + /* Skip current log file temporary file */ > +

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-19 Thread Michael Paquier
On Fri, Jan 20, 2017 at 3:29 AM, Karl O. Pinc wrote: > On Thu, 19 Jan 2017 12:12:18 -0300 > Alvaro Herrera wrote: > >> Karl O. Pinc wrote: >> > On Wed, 18 Jan 2017 19:27:40 -0300 >> > Alvaro Herrera wrote: >> >> > > I thought

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-19 Thread Michael Paquier
On Fri, Jan 20, 2017 at 12:08 AM, Karl O. Pinc wrote: > Is this an argument for having the current_logfiles always exist > and be empty when there is no in-filesystem logfile? It always felt > to me that the code would be simpler that way. Well, you'll need to do something in any

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-19 Thread Karl O. Pinc
On Thu, 19 Jan 2017 12:12:18 -0300 Alvaro Herrera wrote: > Karl O. Pinc wrote: > > On Wed, 18 Jan 2017 19:27:40 -0300 > > Alvaro Herrera wrote: > > > > I thought this part was odd -- I mean, why is SysLogger_Start() > > > being called if

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-19 Thread Alvaro Herrera
Karl O. Pinc wrote: > On Wed, 18 Jan 2017 19:27:40 -0300 > Alvaro Herrera wrote: > > I thought this part was odd -- I mean, why is SysLogger_Start() being > > called if the collector is not enabled? Turns out we do it and return > > early if not enabled. But not in

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-19 Thread Karl O. Pinc
On Wed, 18 Jan 2017 19:27:40 -0300 Alvaro Herrera wrote: > Karl O. Pinc wrote: > > > @@ -511,10 +519,16 @@ int > > SysLogger_Start(void) > > { > > pid_t sysloggerPid; > > - char *filename; > > > > + /* > > +* Logging collector is not

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-19 Thread Karl O. Pinc
On Thu, 19 Jan 2017 16:59:10 +0900 Michael Paquier wrote: > On Thu, Jan 19, 2017 at 7:27 AM, Alvaro Herrera > wrote: > > I don't think the "abstract names" stuff is an improvement (just > > look at the quoting mess in ConfigureNamesString).

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Michael Paquier
On Thu, Jan 19, 2017 at 7:27 AM, Alvaro Herrera wrote: > I thought this part was odd -- I mean, why is SysLogger_Start() being > called if the collector is not enabled? Turns out we do it and return > early if not enabled. But not in all cases -- there is one callsite

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Alvaro Herrera
Karl O. Pinc wrote: > @@ -511,10 +519,16 @@ int > SysLogger_Start(void) > { > pid_t sysloggerPid; > - char *filename; > > + /* > + * Logging collector is not enabled. We don't know where messages are > + * logged. Remove outdated file holding the

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Michael Paquier
On Thu, Jan 19, 2017 at 6:56 AM, Karl O. Pinc wrote: > On Wed, 18 Jan 2017 15:08:09 -0600 > "Karl O. Pinc" wrote: > >> I would like to see index entries for "current_logfiles" >> so this stuff is findable. > > Attached is a v27 of the patch. > > I polished some of

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Michael Paquier
On Thu, Jan 19, 2017 at 6:08 AM, Karl O. Pinc wrote: > On Wed, 18 Jan 2017 15:52:36 -0500 > Robert Haas wrote: > >> On Wed, Jan 18, 2017 at 12:08 PM, Karl O. Pinc wrote: >> > Seems to me that the file format should >> > be documented if

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Karl O. Pinc
On Wed, 18 Jan 2017 15:08:09 -0600 "Karl O. Pinc" wrote: > I would like to see index entries for "current_logfiles" > so this stuff is findable. Attached is a v27 of the patch. I polished some of the sentences in the documentation and added index entries. Also attached are a

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Karl O. Pinc
On Wed, 18 Jan 2017 15:52:36 -0500 Robert Haas wrote: > On Wed, Jan 18, 2017 at 12:08 PM, Karl O. Pinc wrote: > > Seems to me that the file format should > > be documented if there's any intention that the end user > > look at or otherwise use the file's

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Robert Haas
On Wed, Jan 18, 2017 at 12:08 PM, Karl O. Pinc wrote: > Seems to me that the file format should > be documented if there's any intention that the end user > look at or otherwise use the file's content. > > It's fine with me if the content of current_logfiles > is supposed to be

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Robert Haas
On Wed, Jan 18, 2017 at 12:56 PM, Karl O. Pinc wrote: > If it were me I'd have the documentation mention > that the pg_current_logfiles() result is > supplied on a "best effort" basis and under > rare conditions may be outdated. The > sentence in the pg_current_logfles() docs >

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Karl O. Pinc
On Wed, 18 Jan 2017 11:08:23 -0600 "Karl O. Pinc" wrote: > Hi Micheal, > > On Wed, 18 Jan 2017 10:26:43 -0600 > "Karl O. Pinc" wrote: > > > > v26 patch attached which fixes this. > > I was glancing over the changes to the documentation > you made between the

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Karl O. Pinc
Hi Micheal, On Wed, 18 Jan 2017 10:26:43 -0600 "Karl O. Pinc" wrote: > > v26 patch attached which fixes this. I was glancing over the changes to the documentation you made between the v22 and v25 and from looking at the diffs it seems the format of the current_logfiles file

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Karl O. Pinc
On Wed, 18 Jan 2017 10:11:20 -0600 "Karl O. Pinc" wrote: > You must write > errcode(ERRCODE_INTERNAL_ERROR) > instead of just > ERRCODE_INTERNAL_ERROR > > If you don't you get back 01000 for the error code. > v26 patch attached which fixes this. Attached are revised

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-18 Thread Karl O. Pinc
On Wed, 18 Jan 2017 13:26:46 +0900 Michael Paquier wrote: > On Wed, Jan 18, 2017 at 11:36 AM, Karl O. Pinc wrote: > > On Wed, 18 Jan 2017 11:08:25 +0900 > > Michael Paquier wrote: > > > >> Our ideas rather map here,

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-17 Thread Michael Paquier
On Wed, Jan 18, 2017 at 11:36 AM, Karl O. Pinc wrote: > On Wed, 18 Jan 2017 11:08:25 +0900 > Michael Paquier wrote: > >> Our ideas rather map here, ERRCODE_INTERNAL_ERROR would be adapted for >> this situation. Do any of you want to give it a shot or

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-17 Thread Karl O. Pinc
On Wed, 18 Jan 2017 11:08:25 +0900 Michael Paquier wrote: > Our ideas rather map here, ERRCODE_INTERNAL_ERROR would be adapted for > this situation. Do any of you want to give it a shot or should I? You're welcome to it. Karl Free Software: "You

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-17 Thread Michael Paquier
On Wed, Jan 18, 2017 at 7:36 AM, Karl O. Pinc wrote: > Maybe. It's not user-supplied data that's corrupted but it is > PG generated data which is generated for and supplied to the user. > I just looked at all uses of XX001 and it is true that they all > involve corruption of

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-17 Thread Michael Paquier
On Wed, Jan 18, 2017 at 3:06 AM, Gilles Darold wrote: > This have already been discuted previously in this thread, one of my > previous patch version has implemented this behavior but we decide that > what we really want is to be able to use the function using the >

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-17 Thread Karl O. Pinc
On Tue, 17 Jan 2017 23:00:43 +0100 Gilles Darold wrote: > Le 17/01/2017 à 19:58, Karl O. Pinc a écrit : > > On Tue, 17 Jan 2017 19:06:22 +0100 > > Gilles Darold wrote: > > > >> Le 17/01/2017 à 03:22, Michael Paquier a écrit : > >>> On Tue,

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-17 Thread Gilles Darold
Le 17/01/2017 à 19:58, Karl O. Pinc a écrit : > On Tue, 17 Jan 2017 19:06:22 +0100 > Gilles Darold wrote: > >> Le 17/01/2017 à 03:22, Michael Paquier a écrit : >>> On Tue, Jan 17, 2017 at 2:21 AM, Karl O. Pinc >>> wrote: On January 15, 2017

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-17 Thread Karl O. Pinc
On Tue, 17 Jan 2017 19:06:22 +0100 Gilles Darold wrote: > Le 17/01/2017 à 03:22, Michael Paquier a écrit : > > On Tue, Jan 17, 2017 at 2:21 AM, Karl O. Pinc > > wrote: > >> On January 15, 2017 11:47:51 PM CST, Michael Paquier > >>

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-17 Thread Gilles Darold
Le 17/01/2017 à 03:22, Michael Paquier a écrit : > On Tue, Jan 17, 2017 at 2:21 AM, Karl O. Pinc wrote: >> On January 15, 2017 11:47:51 PM CST, Michael Paquier >> wrote: >>> On Mon, Jan 16, 2017 at 1:14 PM, Karl O. Pinc wrote: >>> With

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-17 Thread Karl O. Pinc
On Tue, 17 Jan 2017 07:58:43 -0600 "Karl O. Pinc" wrote: > On Tue, 17 Jan 2017 11:22:45 +0900 > Michael Paquier wrote: > > > On Tue, Jan 17, 2017 at 2:21 AM, Karl O. Pinc > > wrote: > > > >>Also, I would rather see an ERROR returned

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-17 Thread Karl O. Pinc
On Tue, 17 Jan 2017 11:22:45 +0900 Michael Paquier wrote: > On Tue, Jan 17, 2017 at 2:21 AM, Karl O. Pinc wrote: > >>Also, I would rather see an ERROR returned to the user in case of > >>bad data in current_logfiles, I did not change that either as >

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-16 Thread Michael Paquier
On Tue, Jan 17, 2017 at 2:21 AM, Karl O. Pinc wrote: > On January 15, 2017 11:47:51 PM CST, Michael Paquier > wrote: >>On Mon, Jan 16, 2017 at 1:14 PM, Karl O. Pinc wrote: >>With all those issues fixed, I finish with the attached, that I

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-16 Thread Alvaro Herrera
Karl O. Pinc wrote: > I do have a question here regards code formatting. > The patch now contains: > > if (log_filepath == NULL) > { > /* Bad data. Avoid segfaults etc. and return NULL to caller. */ > break; > } > > I'm not sure how this fits in with PG coding style,

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-16 Thread Karl O. Pinc
On January 15, 2017 11:47:51 PM CST, Michael Paquier wrote: >On Mon, Jan 16, 2017 at 1:14 PM, Karl O. Pinc wrote: > > >> Attached also are 2 patches which abstract some hardcoded >> constants into symbols. Whether to apply them is a matter >> of

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-16 Thread Karl O. Pinc
On January 15, 2017 11:47:51 PM CST, Michael Paquier wrote: >On Mon, Jan 16, 2017 at 1:14 PM, Karl O. Pinc wrote: >> I do have a question here regards code formatting. >> The patch now contains: >> >> if (log_filepath == NULL) >> { >>

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-15 Thread Michael Paquier
On Mon, Jan 16, 2017 at 1:14 PM, Karl O. Pinc wrote: > I do have a question here regards code formatting. > The patch now contains: > > if (log_filepath == NULL) > { > /* Bad data. Avoid segfaults etc. and return NULL to caller. */ > break; > } > > I'm

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-15 Thread Karl O. Pinc
On Sun, 15 Jan 2017 07:20:40 -0600 "Karl O. Pinc" wrote: > On Sun, 15 Jan 2017 14:54:44 +0900 > Michael Paquier wrote: > > > I think that's all I have for this patch. > I'd like to submit with it an addendum patch that > makes symbols out of some

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-15 Thread Karl O. Pinc
On Sun, 15 Jan 2017 14:54:44 +0900 Michael Paquier wrote: > I think that's all I have for this patch. I'd like to submit with it an addendum patch that makes symbols out of some constants. I'll see if I can get that done soon. Karl Free Software:

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-15 Thread Gilles Darold
Le 15/01/2017 à 06:54, Michael Paquier a écrit : > On Sat, Jan 14, 2017 at 10:02 PM, Gilles Darold > wrote: >> Le 13/01/2017 à 14:09, Michael Paquier a écrit : >>> On Fri, Jan 13, 2017 at 5:48 PM, Gilles Darold >>> wrote: Le 13/01/2017 à

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-14 Thread Michael Paquier
On Sat, Jan 14, 2017 at 10:02 PM, Gilles Darold wrote: > Le 13/01/2017 à 14:09, Michael Paquier a écrit : >> On Fri, Jan 13, 2017 at 5:48 PM, Gilles Darold >> wrote: >>> Le 13/01/2017 à 05:26, Michael Paquier a écrit : Surely the

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-14 Thread Gilles Darold
Le 13/01/2017 à 14:09, Michael Paquier a écrit : > On Fri, Jan 13, 2017 at 5:48 PM, Gilles Darold > wrote: >> Le 13/01/2017 à 05:26, Michael Paquier a écrit : >>> Surely the temporary file of current_logfiles should not be included >>> in base backups (look at

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-13 Thread Michael Paquier
On Sat, Jan 14, 2017 at 1:43 AM, Kevin Grittner wrote: > On Fri, Jan 13, 2017 at 7:09 AM, Michael Paquier > wrote: >> There is no real harm in including current_logfiles in base >> backups, but that's really in the same bag as postmaster.opts or >>

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-13 Thread Kevin Grittner
On Fri, Jan 13, 2017 at 7:09 AM, Michael Paquier wrote: > There is no real harm in including current_logfiles in base > backups, but that's really in the same bag as postmaster.opts or > postmaster.pid, particularly if the log file name has a > timestamp. I'm going to

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-13 Thread Michael Paquier
On Fri, Jan 13, 2017 at 5:48 PM, Gilles Darold wrote: > Le 13/01/2017 à 05:26, Michael Paquier a écrit : >> Surely the temporary file of current_logfiles should not be included >> in base backups (look at basebackup.c). Documentation needs to reflect >> that as well.

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-13 Thread Gilles Darold
Le 13/01/2017 à 05:26, Michael Paquier a écrit : > OK. I have been looking at this patch. > > git diff --check is very unhappy, particularly because of > update_metainfo_datafile(). Sorry, fixed. > + > +When logs are written to the file-system their paths, names, and > +types are

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-12 Thread Michael Paquier
On Thu, Jan 12, 2017 at 9:14 PM, Gilles Darold wrote: > Le 11/01/2017 à 08:39, Michael Paquier a écrit : >> On Wed, Dec 14, 2016 at 12:19 AM, Robert Haas wrote: >>> On Mon, Dec 12, 2016 at 4:31 AM, Gilles Darold >>>

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-12 Thread Michael Paquier
On Thu, Jan 12, 2017 at 9:55 PM, Karl O. Pinc wrote: > On Thu, 12 Jan 2017 13:14:28 +0100 > Gilles Darold wrote: > >> My bad, I was thinking that Karl has planned an update of the patch in >> his last post, sorry for my misunderstanding. > > I was, but

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-12 Thread Karl O. Pinc
On Thu, 12 Jan 2017 13:14:28 +0100 Gilles Darold wrote: > My bad, I was thinking that Karl has planned an update of the patch in > his last post, sorry for my misunderstanding. I was, but have been swept along by events and not gotten to it. Karl

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-12 Thread Gilles Darold
Le 11/01/2017 à 08:39, Michael Paquier a écrit : > On Wed, Dec 14, 2016 at 12:19 AM, Robert Haas wrote: >> On Mon, Dec 12, 2016 at 4:31 AM, Gilles Darold >> wrote: >>> Applied in last version of the patch v18 as well as removing of an >>> unused

Re: [HACKERS] Patch to implement pg_current_logfile() function

2017-01-10 Thread Michael Paquier
On Wed, Dec 14, 2016 at 12:19 AM, Robert Haas wrote: > On Mon, Dec 12, 2016 at 4:31 AM, Gilles Darold > wrote: >> Applied in last version of the patch v18 as well as removing of an >> unused variable. > > OK, this looks a lot closer to being

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-13 Thread Karl O. Pinc
Hi Gilles, I don't plan to be able to get back to this patch until late this week or early next week. My plan is to then go though the whole thing and fix everything I can find. If we're both working at the same time this could lead to wasted effort so I will write as soon as I start work and

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-13 Thread Robert Haas
On Thu, Dec 8, 2016 at 4:34 PM, Karl O. Pinc wrote: >> I do think that the blizzard of small patches that you've >> submitted in some of your emails may possibly be a bit overwhelming. >> We shouldn't really need a stack of a dozen or more patches to >> implement one small

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-13 Thread Robert Haas
On Mon, Dec 12, 2016 at 4:31 AM, Gilles Darold wrote: > Applied in last version of the patch v18 as well as removing of an > unused variable. OK, this looks a lot closer to being committable. But: @@ -570,11 +583,13 @@ SysLogger_Start(void) * a time-based

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-12 Thread Gilles Darold
Le 11/12/2016 à 04:38, Karl O. Pinc a écrit : > On Sat, 10 Dec 2016 19:41:21 -0600 > "Karl O. Pinc" wrote: > >> On Fri, 9 Dec 2016 23:36:12 -0600 >> "Karl O. Pinc" wrote: >> >>> Instead I propose (code I have not actually executed): >>> ... >>> char

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-10 Thread Karl O. Pinc
On Sat, 10 Dec 2016 19:41:21 -0600 "Karl O. Pinc" wrote: > On Fri, 9 Dec 2016 23:36:12 -0600 > "Karl O. Pinc" wrote: > > > Instead I propose (code I have not actually executed): > > ... > > charlbuffer[MAXPGPATH]; > > char*log_format = lbuffer; > > ... > >

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-10 Thread Karl O. Pinc
On Fri, 9 Dec 2016 23:36:12 -0600 "Karl O. Pinc" wrote: > Instead I propose (code I have not actually executed): > ... > charlbuffer[MAXPGPATH]; > char*log_format = lbuffer; > ... > > /* extract log format and log file path from the line */ > log_filepath =

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-09 Thread Karl O. Pinc
Hi Gilles, On Fri, 9 Dec 2016 23:41:25 +0100 Gilles Darold wrote: > /* extract log format and log file path from the line */ > log_filepath = strchr(lbuffer, ' '); > log_filepath++; >

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-09 Thread Gilles Darold
Le 08/12/2016 à 00:52, Robert Haas a écrit : > On Tue, Dec 6, 2016 at 6:11 PM, Gilles Darold > wrote: >> It seems that all fixes have been included in this patch. > Yikes. This patch has certainly had a lot of review, but it has lots > of basic inconsistencies with

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-08 Thread Karl O. Pinc
On Thu, 8 Dec 2016 11:27:34 -0500 Robert Haas wrote: > On Wed, Dec 7, 2016 at 11:02 PM, Karl O. Pinc wrote: > > I read this and knew that I hadn't finished review, but didn't > > immediately respond because I thought the patch had to be > > marked "ready

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-08 Thread Robert Haas
On Wed, Dec 7, 2016 at 11:02 PM, Karl O. Pinc wrote: > I read this and knew that I hadn't finished review, but didn't > immediately respond because I thought the patch had to be > marked "ready for committer" on commitfest.postgresql.org > before the committers would spend a lot of

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-07 Thread Karl O. Pinc
Hello Robert, On Wed, 7 Dec 2016 18:52:24 -0500 Robert Haas wrote: > On Tue, Dec 6, 2016 at 6:11 PM, Gilles Darold > wrote: > > It seems that all fixes have been included in this patch. I read this and knew that I hadn't finished review, but

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-07 Thread Tom Lane
Robert Haas writes: > Yikes. This patch has certainly had a lot of review, but it has lots > of basic inconsistencies with PostgreSQL coding style, which one would > think somebody would have noticed by now, and there are multiple > places where the logic seems to do in a

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-07 Thread Robert Haas
On Tue, Dec 6, 2016 at 6:11 PM, Gilles Darold wrote: > It seems that all fixes have been included in this patch. Yikes. This patch has certainly had a lot of review, but it has lots of basic inconsistencies with PostgreSQL coding style, which one would think somebody

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-06 Thread Gilles Darold
Le 02/12/2016 à 02:08, Karl O. Pinc a écrit : > On Sun, 27 Nov 2016 21:54:46 +0100 > Gilles Darold wrote: > >> I've attached the v15 of the patch >> I've not applied patch patch_pg_current_logfile-v14.diff.backoff to >> prevent constant call of logfile_writename() on a

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-02 Thread Haribabu Kommi
On Fri, Dec 2, 2016 at 12:08 PM, Karl O. Pinc wrote: > On Sun, 27 Nov 2016 21:54:46 +0100 > Gilles Darold wrote: > > > I've attached the v15 of the patch > > > I've not applied patch patch_pg_current_logfile-v14.diff.backoff to > > prevent constant call

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-12-01 Thread Karl O. Pinc
On Sun, 27 Nov 2016 21:54:46 +0100 Gilles Darold wrote: > I've attached the v15 of the patch > I've not applied patch patch_pg_current_logfile-v14.diff.backoff to > prevent constant call of logfile_writename() on a busy system (errno = > ENFILE | EMFILE). I don't

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-29 Thread Karl O. Pinc
Hi Gilles, On Sun, 27 Nov 2016 21:54:46 +0100 Gilles Darold wrote: > I've attached the v15 of the patch that applies your changes/fixes ... Per the following: On Mon, 21 Nov 2016 13:17:17 -0500 Robert Haas wrote: > It would really be much

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-28 Thread Gilles Darold
Le 28/11/2016 à 18:54, Karl O. Pinc a écrit : > Hi Gilles, > > On Sun, 27 Nov 2016 21:54:46 +0100 > Gilles Darold wrote: > >> I've attached the v15 of the patch that applies your changes/fixes and >> remove the call to strtok(). > I like the idea of replacing the

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-28 Thread Karl O. Pinc
Hi Gilles, On Sun, 27 Nov 2016 21:54:46 +0100 Gilles Darold wrote: > I've attached the v15 of the patch that applies your changes/fixes and > remove the call to strtok(). I like the idea of replacing the strtok() call with sscanf() but I see problems. It won't work

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-27 Thread Karl O. Pinc
On Sun, 27 Nov 2016 21:54:46 +0100 Gilles Darold wrote: > Again, patches patch_pg_current_logfile-v14.diff.doc_linux_default-v2 > have not been included because I don't see any reason to talk > especially about systemd. If you talk about systemd you must talk > about

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-27 Thread Gilles Darold
Hi Karl, I've attached the v15 of the patch that applies your changes/fixes and remove the call to strtok(). I've not applied patch patch_pg_current_logfile-v14.diff.backoff to prevent constant call of logfile_writename() on a busy system (errno = ENFILE | EMFILE). I think this can be done quite

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-24 Thread Karl O. Pinc
On Wed, 23 Nov 2016 23:08:18 -0600 "Karl O. Pinc" wrote: > On Wed, 23 Nov 2016 03:21:05 -0600 > "Karl O. Pinc" wrote: > > > On Sat, 19 Nov 2016 12:58:47 +0100 > > Gilles Darold wrote: > > > > > ... attached v14 of the patch. > >

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-23 Thread Karl O. Pinc
On Wed, 23 Nov 2016 03:21:05 -0600 "Karl O. Pinc" wrote: > On Sat, 19 Nov 2016 12:58:47 +0100 > Gilles Darold wrote: > > > ... attached v14 of the patch. > patch_pg_current_logfile-v14.diff.retry_current_logfiles-part3 > Re-try the write of

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-23 Thread Karl O. Pinc
On Wed, 23 Nov 2016 10:39:28 -0500 Tom Lane wrote: > Robert Haas writes: > > On Wed, Nov 23, 2016 at 4:21 AM, Karl O. Pinc > > wrote: > >> The bool types on the stack in logfile_rotate() are > >> my work. Bools on the stack don't

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-23 Thread Gilles Darold
Le 23/11/2016 à 16:26, Tom Lane a écrit : > "Karl O. Pinc" writes: >> Maybe on the 2nd call to strtok() you could pass "" >> as the 2nd argument? That'd be a little wonky but >> the man page does not say you can't have an empty >> set of delimiters. On the other hand strtok() is

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-23 Thread Tom Lane
Robert Haas writes: > On Wed, Nov 23, 2016 at 4:21 AM, Karl O. Pinc wrote: >> The bool types on the stack in logfile_rotate() are >> my work. Bools on the stack don't make sense as far >> as hardware goes, so the compiler's optimizer should change >> them

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-23 Thread Robert Haas
On Wed, Nov 23, 2016 at 4:21 AM, Karl O. Pinc wrote: > patch_pg_current_logfile-v14.diff.bool_to_int > > Do not include in "main" patch, submit to maintainers separately. > > Applies on top of patch_pg_current_logfile-v14.diff > > The bool types on the stack in logfile_rotate() are

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-23 Thread Tom Lane
"Karl O. Pinc" writes: > Maybe on the 2nd call to strtok() you could pass "" > as the 2nd argument? That'd be a little wonky but > the man page does not say you can't have an empty > set of delimiters. On the other hand strtok() is > not a perfect choice, you don't want to

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-23 Thread Karl O. Pinc
On Wed, 23 Nov 2016 03:21:05 -0600 "Karl O. Pinc" wrote: > But also, you can't use strtok() to parse lbuffer because > the path you're returning can contain a space. Maybe on the 2nd call to strtok() you could pass "" as the 2nd argument? That'd be a little wonky but the man

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-23 Thread Karl O. Pinc
Hi Gilles, On Sat, 19 Nov 2016 12:58:47 +0100 Gilles Darold wrote: > ... attached v14 of the patch. Attached are patches for your consideration and review. (Including your latest v14 patch for completeness.) Some of the attached patches (like the GUC symbol patch

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-21 Thread Karl O. Pinc
On Mon, 21 Nov 2016 13:17:17 -0500 Robert Haas wrote: > > I've a couple of other patches that do > > a little cleanup on master that I'd also like to submit > > along with your patch. > It would really be much better to submit anything that's not > absolutely necessary

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-21 Thread Robert Haas
On Sat, Nov 19, 2016 at 8:51 AM, Karl O. Pinc wrote: > On Sat, 19 Nov 2016 12:58:47 +0100 > Gilles Darold wrote: > >> All patches you've submitted on tha v13 patch have been applied and >> are present in attached v14 of the patch. I have not included the

Re: [HACKERS] Patch to implement pg_current_logfile() function

2016-11-19 Thread Karl O. Pinc
On Sat, 19 Nov 2016 18:59:49 +0100 Gilles Darold wrote: > Le 19/11/2016 à 16:22, Karl O. Pinc a écrit : > > Hi Gilles, > > > > On Tue, 15 Nov 2016 15:15:52 -0600 > > "Karl O. Pinc" wrote: > > > >>> On Mon, 7 Nov 2016 23:29:28 +0100 > >>> Gilles Darold

  1   2   >