Thread: pg_ctl using START with paths needing quotes

pg_ctl using START with paths needing quotes

From
Bruce Momjian
Date:
This applied patch changes the way pg_ctl starts on Win32.

Using START, it is not possible to quote the executable name, who's
directory might have spaces:

    START /B /program files/x.exe

The fix is to create a temporary batch file in C:\ containing:

    /program files/x.exe

and run START with the batch name:

    START /B C:\PG_CTL_323223.BAT

then unlink the batch file.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/bin/pg_ctl/pg_ctl.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.13
diff -c -c -r1.13 pg_ctl.c
*** src/bin/pg_ctl/pg_ctl.c    10 Jun 2004 22:20:53 -0000    1.13
--- src/bin/pg_ctl/pg_ctl.c    11 Jun 2004 00:56:22 -0000
***************
*** 221,226 ****
--- 221,261 ----
       * to pass everything to a shell to process them.
       */
      char        cmd[MAXPGPATH];
+     int            ret;
+     char        *pgexec = postgres_path;
+
+ #ifdef WIN32
+     /*
+      *    Win32 has a problem with the interaction between START and system().
+      *    There is no way to quote the executable name passed to START.
+      *    Therefore, we put the executable name in a temporary batch file
+      *    and run it via START.
+      */
+     char        tmp[MAXPGPATH] = "C:\\PG_CTL_XXXXXX", /* good location? */
+                 bat[MAXPGPATH];
+     int            fd = mkstemp(tmp);
+
+     if (fd == -1)
+     {
+         fprintf(stderr, _("%s: cannot create batch file %s to start server: %s\n"),
+                         progname, tmp, strerror(errno));
+         exit(1);
+     }
+     write(fd, postgres_path, strlen(postgres_path));
+     write(fd, "\n", 1);
+     close(fd);
+
+     strcpy(bat, tmp);
+     strcat(bat, ".BAT");
+     pgexec = bat;
+     if (rename(tmp, bat) == -1)
+     {
+         fprintf(stderr, _("%s: cannot rename batch file %s to %s: %s\n"),
+                         progname, tmp, bat, strerror(errno));
+         unlink(tmp);
+         exit(1);
+     }
+ #endif

      if (log_file != NULL)
          /* Win32 needs START /B rather than "&" */
***************
*** 229,235 ****
  #else
          snprintf(cmd, MAXPGPATH, "%sSTART /B \"%s\" %s < \"%s\" >> \"%s\" 2>&1%s",
  #endif
!                  SYSTEMQUOTE, postgres_path, post_opts, DEVNULL, log_file,
                   SYSTEMQUOTE);
      else
  #ifndef WIN32
--- 264,270 ----
  #else
          snprintf(cmd, MAXPGPATH, "%sSTART /B \"%s\" %s < \"%s\" >> \"%s\" 2>&1%s",
  #endif
!                  SYSTEMQUOTE, pgexec, post_opts, DEVNULL, log_file,
                   SYSTEMQUOTE);
      else
  #ifndef WIN32
***************
*** 237,244 ****
  #else
          snprintf(cmd, MAXPGPATH, "%sSTART /B \"%s\" %s < \"%s\" 2>&1%s",
  #endif
!                  SYSTEMQUOTE, postgres_path, post_opts, DEVNULL, SYSTEMQUOTE);
!     return system(cmd);
  }


--- 272,291 ----
  #else
          snprintf(cmd, MAXPGPATH, "%sSTART /B \"%s\" %s < \"%s\" 2>&1%s",
  #endif
!                  SYSTEMQUOTE, pgexec, post_opts, DEVNULL, SYSTEMQUOTE);
!
!     ret = system(cmd);
!
! #ifdef WIN32
!     /* We are unlinking it while it is running, but that should be OK */
!     if (unlink(bat))
!     {
!         fprintf(stderr, _("%s: cannot remove batch file %s: %s\n"),
!                         progname, bat, strerror(errno));
!         exit(1);
!     }
! #endif
!     return ret;
  }



Re: pg_ctl using START with paths needing quotes

From
Andrew Dunstan
Date:
This is a really ugly hack (I take the blame since I gave Bruce the
idea). There are a few things to note:

. the .bat file should probably be created in the data dir - that's
about the only place that we should be guaranteed we can write.
. the command should be preceded by '@' to suppress echoing
. the whole command, including redirections should go inside the .bat
file, so that pg_ctl just issues 'start /b foo.bat'

There are still things to clean up in pg_ctl, e.g. its handling of
relative paths to the data dir.

cheers

andrew



Bruce Momjian wrote:

>This applied patch changes the way pg_ctl starts on Win32.
>
>Using START, it is not possible to quote the executable name, who's
>directory might have spaces:
>
>    START /B /program files/x.exe
>
>The fix is to create a temporary batch file in C:\ containing:
>
>    /program files/x.exe
>
>and run START with the batch name:
>
>    START /B C:\PG_CTL_323223.BAT
>
>then unlink the batch file.
>
>
>

Fix for erroneous warning on Shutdown

From
Simon Riggs
Date:
Minor patch to correct erroneous warning in cvs tip, believed to be a
very minor regression.

When a shutdown was requested within CHECKPOINT_SECONDS of a checkpoint,
the shutdown code in the bgwriter (which has does checkpointing) would
issue the erroneous message:

LOG:  checkpoints are occurring too frequently (%d seconds apart)
HINT:  Consider increasing the configuration parameter
"checkpoint_segments".

Clearly, this should only occur when specific checkpoint requests have
been made, shutdown checkpoints should not be included in the warning.

Best regards, Simon Riggs

Attachment

Re: pg_ctl using START with paths needing quotes

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
>
> This is a really ugly hack (I take the blame since I gave Bruce the
> idea). There are a few things to note:
>
> . the .bat file should probably be created in the data dir - that's
> about the only place that we should be guaranteed we can write.
> . the command should be preceded by '@' to suppress echoing
> . the whole command, including redirections should go inside the .bat
> file, so that pg_ctl just issues 'start /b foo.bat'
>
> There are still things to clean up in pg_ctl, e.g. its handling of
> relative paths to the data dir.

Hack removed.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: pg_ctl using START with paths needing quotes

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> Bruce Momjian wrote:
>> This applied patch changes the way pg_ctl starts on Win32.
>>
>> Using START, it is not possible to quote the executable name, who's
>> directory might have spaces:
>
> This is a really ugly hack (I take the blame since I gave Bruce the
> idea). There are a few things to note:

> . the .bat file should probably be created in the data dir - that's
> about the only place that we should be guaranteed we can write.

In that case, haven't we simply moved the spaces problem from the
executable directory to the data directory?

            regards, tom lane

Re: pg_ctl using START with paths needing quotes

From
Andrew Dunstan
Date:
Tom Lane wrote:

>Andrew Dunstan <andrew@dunslane.net> writes:
>
>
>>Bruce Momjian wrote:
>>
>>
>>>This applied patch changes the way pg_ctl starts on Win32.
>>>
>>>Using START, it is not possible to quote the executable name, who's
>>>directory might have spaces:
>>>
>>>
>>This is a really ugly hack (I take the blame since I gave Bruce the
>>idea). There are a few things to note:
>>
>>
>
>
>
>>. the .bat file should probably be created in the data dir - that's
>>about the only place that we should be guaranteed we can write.
>>
>>
>
>In that case, haven't we simply moved the spaces problem from the
>executable directory to the data directory?
>
>
>

Yes. you're right. The hack is gone now so it's moot.

cheers

andrew

Re: pg_ctl using START with paths needing quotes

From
Bruce Momjian
Date:
Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > Bruce Momjian wrote:
> >> This applied patch changes the way pg_ctl starts on Win32.
> >>
> >> Using START, it is not possible to quote the executable name, who's
> >> directory might have spaces:
> >
> > This is a really ugly hack (I take the blame since I gave Bruce the
> > idea). There are a few things to note:
>
> > . the .bat file should probably be created in the data dir - that's
> > about the only place that we should be guaranteed we can write.
>
> In that case, haven't we simply moved the spaces problem from the
> executable directory to the data directory?

Yep.  That code is all gone now that we have the right fix, use:

     START "" "executable"

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: Fix for erroneous warning on Shutdown

From
Tom Lane
Date:
Simon Riggs <simon@2ndquadrant.com> writes:
> Minor patch to correct erroneous warning in cvs tip, believed to be a
> very minor regression.

This patch is wrong; it effectively disables the warning altogether.

> When a shutdown was requested within CHECKPOINT_SECONDS of a checkpoint,
> the shutdown code in the bgwriter (which has does checkpointing) would
> issue the erroneous message:

I don't think so ... the shutdown path doesn't go through this code.
Could you take another look at the cause of what you saw?

            regards, tom lane

Re: Fix for erroneous warning on Shutdown

From
Bruce Momjian
Date:
Patch rejected, asking for more research.

---------------------------------------------------------------------------

Simon Riggs wrote:
>
> Minor patch to correct erroneous warning in cvs tip, believed to be a
> very minor regression.
>
> When a shutdown was requested within CHECKPOINT_SECONDS of a checkpoint,
> the shutdown code in the bgwriter (which has does checkpointing) would
> issue the erroneous message:
>
> LOG:  checkpoints are occurring too frequently (%d seconds apart)
> HINT:  Consider increasing the configuration parameter
> "checkpoint_segments".
>
> Clearly, this should only occur when specific checkpoint requests have
> been made, shutdown checkpoints should not be included in the warning.
>
> Best regards, Simon Riggs

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>       subscribe-nomail command to majordomo@postgresql.org so that your
>       message can get through to the mailing list cleanly

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: pg_ctl using START with paths needing quotes

From
Andrew Dunstan
Date:
Bruce Momjian wrote:

>Tom Lane wrote:
>
>
>>Andrew Dunstan <andrew@dunslane.net> writes:
>>
>>
>>>Bruce Momjian wrote:
>>>
>>>
>>>>This applied patch changes the way pg_ctl starts on Win32.
>>>>
>>>>Using START, it is not possible to quote the executable name, who's
>>>>directory might have spaces:
>>>>
>>>>
>>>This is a really ugly hack (I take the blame since I gave Bruce the
>>>idea). There are a few things to note:
>>>
>>>
>>>. the .bat file should probably be created in the data dir - that's
>>>about the only place that we should be guaranteed we can write.
>>>
>>>
>>In that case, haven't we simply moved the spaces problem from the
>>executable directory to the data directory?
>>
>>
>
>Yep.  That code is all gone now that we have the right fix, use:
>
>     START "" "executable"
>
>
>

For the record, and in case we need to use it in future, here's what I
got working in pg_ctl.c for starting without any shell call required
(lacks error checking).

cheers

andrew


#ifdef WIN32

    char exepath[MAXPGPATH];
    STARTUPINFO si;
    PROCESS_INFORMATION pi;
    bool rval;
    int null_fd, log_fd;
    int save_stdin, save_stdout, save_stderr;

    save_stdin = _dup(fileno(stdin));

    ZeroMemory(&si,sizeof(STARTUPINFO));
    si.cb = sizeof(STARTUPINFO);

    null_fd = open("nul",O_RDONLY,0);
    dup2(null_fd, fileno(stdin));
    if (log_file != NULL)
      {
        save_stdout = _dup(fileno(stdout));
        save_stderr = _dup(fileno(stderr));
        log_fd = open(log_file,O_WRONLY|O_CREAT|O_APPEND,0700);
        dup2(log_fd, fileno(stdout));
        dup2(log_fd ,fileno(stderr));
      }

    snprintf(exepath,MAXPGPATH,"%s",postgres_path);
    snprintf(cmd,MAXPGPATH,"\"%s\" %s",postgres_path,post_opts);
    rval = CreateProcess(exepath,cmd,NULL,NULL,true,0,NULL,NULL,&si,&pi);
    if (rval == 0)
      {
        CloseHandle(pi.hThread);
        CloseHandle(pi.hProcess);
      }
    dup2(save_stdin, fileno(stdin));
    close(null_fd);
    if (log_file != NULL)
      {
        dup2(save_stdout,fileno(stdout));
        dup2(save_stderr,fileno(stderr));
        close(log_fd);
      }
    return (rval == 0);

#else

Re: Fix for erroneous warning on Shutdown

From
Simon Riggs
Date:
On Fri, 2004-06-11 at 19:25, Tom Lane wrote:
> Simon Riggs <simon@2ndquadrant.com> writes:
> > Minor patch to correct erroneous warning in cvs tip, believed to be a
> > very minor regression.
>
> This patch is wrong; it effectively disables the warning altogether.
>
> > When a shutdown was requested within CHECKPOINT_SECONDS of a checkpoint,
> > the shutdown code in the bgwriter (which has does checkpointing) would
> > issue the erroneous message:
>
> I don't think so ... the shutdown path doesn't go through this code.
> Could you take another look at the cause of what you saw?

Hmmm...perhaps I should submit this as a *low priority* bug then, rather
than just a patch?

As of now, (i.e. even including the new bgwriter shutdown) if you:
1. start postmaster
2. do some work that writes xlog
3. shutdown within some few seconds of startup

you get a WARNING suggesting you increase CHECKPOINT_SEGMENTS, which is
clearly erroneous since no checkpoint has taken place since startup.

Not exactly a common circumstance, I grant you, but it does seem to be a
regression nonetheless. My thinking was that the circumstance was not
limited to this edge case, but actually any shutdown within 30s of a
checkpoint, but the latter is just speculation.

Only reason I spotted it is I've been doing a lot of startup/shutdown
work recently...

Best Regards, Simon Riggs



Re: Fix for erroneous warning on Shutdown

From
Tom Lane
Date:
Simon Riggs <simon@2ndquadrant.com> writes:
> As of now, (i.e. even including the new bgwriter shutdown) if you:
> 1. start postmaster
> 2. do some work that writes xlog
> 3. shutdown within some few seconds of startup
> you get a WARNING suggesting you increase CHECKPOINT_SEGMENTS, which is
> clearly erroneous since no checkpoint has taken place since startup.

[ scratches head... ]  Not for me.  Are you using any nondefault
settings in postgresql.conf?

            regards, tom lane

Re: Fix for erroneous warning on Shutdown

From
Simon Riggs
Date:
On Tue, 2004-06-15 at 19:33, Tom Lane wrote:
> Simon Riggs <simon@2ndquadrant.com> writes:
> > As of now, (i.e. even including the new bgwriter shutdown) if you:
> > 1. start postmaster
> > 2. do some work that writes xlog
> > 3. shutdown within some few seconds of startup
> > you get a WARNING suggesting you increase CHECKPOINT_SEGMENTS, which is
> > clearly erroneous since no checkpoint has taken place since startup.
>
> [ scratches head... ]  Not for me.  Are you using any nondefault
> settings in postgresql.conf?

Hmmm...I did a complete re-download of CVS, then a vanilla
configure/make/make install.... with .conf straight from can. I wanted
to be absolutely certain I had got it clean.

Shutdown was via CTRL-C....make a difference?

Damn I hate the wierd ones...if only because of the 90% chance its
something wrong that I've done...

I'll do it again more slowly and report back...

Best Regards, Simon Riggs



Re: Fix for erroneous warning on Shutdown

From
Tom Lane
Date:
Simon Riggs <simon@2ndquadrant.com> writes:
> Shutdown was via CTRL-C....make a difference?

Wouldn't think so.

I can force the message to appear if I do a *manual* CHECKPOINT command
within thirty seconds of startup.  I'm not sure if that should be
considered wrong or not, but in any case it doesn't seem to be what
you're describing.

            regards, tom lane

Re: Fix for erroneous warning on Shutdown

From
Andrew Dunstan
Date:
Tom Lane wrote:

>Simon Riggs <simon@2ndquadrant.com> writes:
>
>
>>Shutdown was via CTRL-C....make a difference?
>>
>>
>
>Wouldn't think so.
>
>I can force the message to appear if I do a *manual* CHECKPOINT command
>within thirty seconds of startup.  I'm not sure if that should be
>considered wrong or not, but in any case it doesn't seem to be what
>you're describing.
>
>
>

Just to confirm that Simon is not suffering this uniquely, I saw this
the other day on Windows, I believe - meant to report it but it got away
from me.

cheers

andrew

Re: Fix for erroneous warning on Shutdown

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> Just to confirm that Simon is not suffering this uniquely, I saw this
> the other day on Windows, I believe - meant to report it but it got away
> from me.

Oh, I bet I know what's going on --- are you guys launching the
postmaster in a console window and then typing ^C to shut it down?
Depending on your shell that might result in SIGINT being delivered
not only to the postmaster but to all its children.  Backends, if
any, will take that as a query-cancel while the bgwriter will take
it as a checkpoint request.  Which could easily result in the
too-frequent-checkpoints message.

The only real fix I can see for this is to use some other signal than
SIGINT to transmit checkpoint requests to the bgwriter.  Maybe we could
piggyback all bgwriter requests onto SIGUSR2, comparable to the way that
signals up to the postmaster are now handled.  Not sure how important
it is though.

            regards, tom lane

Re: Fix for erroneous warning on Shutdown

From
Simon Riggs
Date:
On Tue, 2004-06-15 at 21:04, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > Just to confirm that Simon is not suffering this uniquely, I saw this
> > the other day on Windows, I believe - meant to report it but it got away
> > from me.
>
> Oh, I bet I know what's going on --- are you guys launching the
> postmaster in a console window and then typing ^C to shut it down?
> Depending on your shell that might result in SIGINT being delivered
> not only to the postmaster but to all its children.  Backends, if
> any, will take that as a query-cancel while the bgwriter will take
> it as a checkpoint request.  Which could easily result in the
> too-frequent-checkpoints message.
>
> The only real fix I can see for this is to use some other signal than
> SIGINT to transmit checkpoint requests to the bgwriter.  Maybe we could
> piggyback all bgwriter requests onto SIGUSR2, comparable to the way that
> signals up to the postmaster are now handled.  Not sure how important
> it is though.
>

Sounds like the cause to me...that would explain why it has just started
happening...

Its minor then... I would only do that for testing...

Regards, Simon



Re: Fix for erroneous warning on Shutdown

From
Bruce Momjian
Date:
Tom Lane wrote:
> Simon Riggs <simon@2ndquadrant.com> writes:
> > Shutdown was via CTRL-C....make a difference?
>
> Wouldn't think so.
>
> I can force the message to appear if I do a *manual* CHECKPOINT command
> within thirty seconds of startup.  I'm not sure if that should be
> considered wrong or not, but in any case it doesn't seem to be what
> you're describing.

It is wrong.  The message should not appear if you issue CHECKPOINT.

I just did CHECKPOINT;CHECKPOINT and got the warning in the logs.  This
needs to be fixed.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: Fix for erroneous warning on Shutdown

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I just did CHECKPOINT;CHECKPOINT and got the warning in the logs.  This
> needs to be fixed.

See code:

            /*
             * Ideally we should only warn if this checkpoint was
             * requested due to running out of segment files, and not
             * if it was manually requested.  However we can't tell the
             * difference with the current signalling mechanism.
             */

I could argue that a client-driven process that issues CHECKPOINT every
few seconds is equally deserving of a warning.  The only thing wrong is
that the HINT is inapplicable ... but that's why it's a HINT and not
part of the main message.

            regards, tom lane

Re: Fix for erroneous warning on Shutdown

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > I just did CHECKPOINT;CHECKPOINT and got the warning in the logs.  This
> > needs to be fixed.
>
> See code:
>
>             /*
>              * Ideally we should only warn if this checkpoint was
>              * requested due to running out of segment files, and not
>              * if it was manually requested.  However we can't tell the
>              * difference with the current signalling mechanism.
>              */
>
> I could argue that a client-driven process that issues CHECKPOINT every
> few seconds is equally deserving of a warning.  The only thing wrong is
> that the HINT is inapplicable ... but that's why it's a HINT and not
> part of the main message.

Also consider they could have issued a checkpoint right after the system
did one.  Yuck.

When I added the warning I hoped to only have it happen for full logs
and not CHECKPOINT, but I guess I couldn't and someone else realized
that and added that clearer comment, or originally I could do that, but
since it has been moved into the bgwriter, it can't anymore.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: Fix for erroneous warning on Shutdown

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> I could argue that a client-driven process that issues CHECKPOINT every
>> few seconds is equally deserving of a warning.  The only thing wrong is
>> that the HINT is inapplicable ... but that's why it's a HINT and not
>> part of the main message.

> Also consider they could have issued a checkpoint right after the system
> did one.  Yuck.

> When I added the warning I hoped to only have it happen for full logs
> and not CHECKPOINT, but I guess I couldn't and someone else realized
> that and added that clearer comment, or originally I could do that, but
> since it has been moved into the bgwriter, it can't anymore.

I believe the original implementation in the postmaster had a somewhat
different set of bugs ;-).  IIRC it did not react to manual checkpoints
but it did confuse WAL checkpoints with timeout-driven checkpoints.
The present bgwriter can distinguish the third but not the first two.

If we were willing to take the time to generalize the
backend-to-bgwriter signaling mechanism then we could distinguish
WAL-driven checkpoints from manually issued checkpoints.  I'm sort of
intending to do that anyway.  The question stands though: why isn't it
appropriate to warn of overly-frequently-issued manual checkpoints?

            regards, tom lane

Re: Fix for erroneous warning on Shutdown

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom Lane wrote:
> >> I could argue that a client-driven process that issues CHECKPOINT every
> >> few seconds is equally deserving of a warning.  The only thing wrong is
> >> that the HINT is inapplicable ... but that's why it's a HINT and not
> >> part of the main message.
>
> > Also consider they could have issued a checkpoint right after the system
> > did one.  Yuck.
>
> > When I added the warning I hoped to only have it happen for full logs
> > and not CHECKPOINT, but I guess I couldn't and someone else realized
> > that and added that clearer comment, or originally I could do that, but
> > since it has been moved into the bgwriter, it can't anymore.
>
> I believe the original implementation in the postmaster had a somewhat
> different set of bugs ;-).  IIRC it did not react to manual checkpoints
> but it did confuse WAL checkpoints with timeout-driven checkpoints.
> The present bgwriter can distinguish the third but not the first two.
>
> If we were willing to take the time to generalize the
> backend-to-bgwriter signaling mechanism then we could distinguish
> WAL-driven checkpoints from manually issued checkpoints.  I'm sort of
> intending to do that anyway.  The question stands though: why isn't it
> appropriate to warn of overly-frequently-issued manual checkpoints?

Imagine pgbench issuing a checkpoint, which it does.  That could trigger
the warning when the parameter really shouldn't be increased.  To me,
the warning is for cases when you are filling up the WAL logs too
quickly and checkpoints are happening too frequently.  If a user is
doing checkpoints, it isn't anything increasing the checkpoint segments
is going to help.

If you can't fix the code to distinguish between manual and wal-full
checkpoints, we can adjust the warning logic to warn when there are 3
checkpoints in a short period, rather than just two.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: Fix for erroneous warning on Shutdown

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> ... The question stands though: why isn't it
>> appropriate to warn of overly-frequently-issued manual checkpoints?

> ... the warning is for cases when you are filling up the WAL logs too
> quickly and checkpoints are happening too frequently.  If a user is
> doing checkpoints, it isn't anything increasing the checkpoint segments
> is going to help.

No, I think the warning is for when checkpoints are happening too
frequently, period.  An overly small checkpoint_segments setting
is one possible cause of that, but the performance penalty from
too many checkpoints is just as bad no matter what's causing it.
(Remember that a checkpoint not only forces I/O in itself, but
significantly increases subsequent WAL traffic because of needing
to dump whole page images into WAL.)

How do you feel about improving the signaling mechanism but using
it just to vary the HINT?

LOG: checkpoints are occurring too frequently (nn seconds apart)
HINT: Consider increasing the configuration parameter "checkpoint_segments".

LOG: checkpoints are occurring too frequently (nn seconds apart)
HINT: Issuing explicit CHECKPOINTs so often is really expensive.

and so on.

            regards, tom lane

Re: Fix for erroneous warning on Shutdown

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom Lane wrote:
> >> ... The question stands though: why isn't it
> >> appropriate to warn of overly-frequently-issued manual checkpoints?
>
> > ... the warning is for cases when you are filling up the WAL logs too
> > quickly and checkpoints are happening too frequently.  If a user is
> > doing checkpoints, it isn't anything increasing the checkpoint segments
> > is going to help.
>
> No, I think the warning is for when checkpoints are happening too
> frequently, period.  An overly small checkpoint_segments setting
> is one possible cause of that, but the performance penalty from
> too many checkpoints is just as bad no matter what's causing it.
> (Remember that a checkpoint not only forces I/O in itself, but
> significantly increases subsequent WAL traffic because of needing
> to dump whole page images into WAL.)
>
> How do you feel about improving the signaling mechanism but using
> it just to vary the HINT?
>
> LOG: checkpoints are occurring too frequently (nn seconds apart)
> HINT: Consider increasing the configuration parameter "checkpoint_segments".
>
> LOG: checkpoints are occurring too frequently (nn seconds apart)
> HINT: Issuing explicit CHECKPOINTs so often is really expensive.

Sure, fine by me.  My only point is that we need something to tell
people they need to increase their checkpoint_segments.  If we add other
warnings, that is fine too.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073