On Sat, Aug 17, 2019 at 10:55:11PM +0200, Adam Wolk wrote:
> Hi ports@,
>
> I'm attaching a small diff to allow picking a different postgresql
> cluster (the directory you create with initdb) location within
> /etc/rc.conf.local.
>
> With the attached diff, one can now define the folder with:
> postgresql_flags="-D /var/postgresql/testdata -w -l
> /var/postgresql/logfile"
>
> Changing the value after -D to point to any valid pg cluster.
>
> The current implementation had the path hard-coded in the rc.d file with
> no way to override it.
>
> By using the existing daemon_flags and setting -D there we indeed
> could start postgresql with a different data_directory as the last -D
> passed to start takes priority. However since that flag was not used in
> the remaining calls the rc script would fail to check, reload and stop a
> postgresql instance started this way.
>
> In order to allow setting a custom data_directory, we remove datadir
> variable and set -D directly in daemon_flags then append ${daemon_flags}
> to all invocations of pg_ctl ($rcexec)) in check, reload, start and stop
> targets. The existing flags we use 'by default' which is -w and -l that
> due to this change are now added to all invocations have the following
> impact:
>
> 1. `-l` ignored in all targets except start, it sets the location of the
> log file. adding it to other commands is a no-op and doesn't change
> existing behavior.
> 2. `-w` affects the start and stop targets. No change in behavior for
> the start target. For the stop target it's also a no-op as reading the
> pg_ctl code - that's the default even if not specified on the command
> line.
>
> Since we got rid of ${datadir} we now have no way to check where is the
> postmaster.pid that the start target attempts to remove. I investigated
> this and came to the conclusion that removing the pidfile is not needed.
>
> When starting, pg_ctl upon noticing that a postmaster.pid is already
> present will issue a warning:
>
> pg_ctl: another server might be running; trying to start server anyway
>
> The pg_ctl code itself handles cases of a leftover pid:
>
> https://github.com/postgres/postgres/blob/b8f2da0ac5a24f669c8d320c58646759b8fc69a5/src/bin/pg_ctl/pg_ctl.c#L583
>
> So if the database starts up properly, the file will be overwritten but
> a system administrator will be able to see a log entry indicating that a
> dead pidfile was removed. Note that pg_ctl stop itself removes the
> postmaster.pid so if it's there then likely the database was not stopped
> properly (and might need investigation for recovery).
>
> The removal of the postmaster.pid has been moved from post server stop
> to server start 9 years ago in commit 1.3. It was present in stop since
> revision 1.1 of the file.
>
> I don't think the above changes require a port quirk. Worst case
> scenario iff someone defined postgresql_flags in rc.conf.local their
> database would now not start due to -D being likely not specified by the
> user. However, this is not permanent (or any whatsoever) damage as the
> user only needs to add -D /var/postgresql/data to his flags to regain
> full functionality.
>
> I do think that this could warrant an entry in current.html. The second
> diff addresses this.
>
> I also noticed that the postgresql_flags line is used verbatim in the
> ENVIRONMENT section of rc.d(8), should I update that manual to include
> the -D that is now the new default for flags? I assume that yes, so the
> third diff covers that, however the extended line wraps in the manual
> now (perhaps that is undesired?).
>
> As always I'm looking for feedback and OK's :)
I like this.
>
> Regards,
> Adam
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/databases/postgresql/Makefile,v
> retrieving revision 1.255
> diff -u -p -r1.255 Makefile
> --- Makefile 12 Aug 2019 16:40:40 -0000 1.255
> +++ Makefile 17 Aug 2019 20:42:47 -0000
> @@ -9,6 +9,7 @@ COMMENT-pg_upgrade=Support for upgrading
>
> VERSION= 11.5
> PREV_MAJOR= 10
> +REVISION-server=1
> DISTNAME= postgresql-${VERSION}
> PKGNAME-main= postgresql-client-${VERSION}
> PKGNAME-server= postgresql-server-${VERSION}
> Index: pkg/postgresql.rc
> ===================================================================
> RCS file: /cvs/ports/databases/postgresql/pkg/postgresql.rc,v
> retrieving revision 1.12
> diff -u -p -r1.12 postgresql.rc
> --- pkg/postgresql.rc 11 Jan 2018 19:27:01 -0000 1.12
> +++ pkg/postgresql.rc 17 Aug 2019 20:42:47 -0000
> @@ -2,10 +2,8 @@
> #
> # $OpenBSD: postgresql.rc,v 1.12 2018/01/11 19:27:01 rpe Exp $
>
> -datadir="/var/postgresql/data"
> -
> daemon="${TRUEPREFIX}/bin/pg_ctl"
> -daemon_flags="-w -l /var/postgresql/logfile"
> +daemon_flags="-D /var/postgresql/data -w -l /var/postgresql/logfile"
> daemon_user="_postgresql"
>
> . /etc/rc.d/rc.subr
> @@ -13,21 +11,20 @@ daemon_user="_postgresql"
> rc_usercheck=NO
>
> rc_check() {
> - ${rcexec} "${daemon} -D ${datadir} status"
> + ${rcexec} "${daemon} status ${daemon_flags}"
> }
>
> rc_reload() {
> - ${rcexec} "${daemon} -D ${datadir} reload"
> + ${rcexec} "${daemon} reload ${daemon_flags}"
> }
>
> rc_start() {
> - rm -f ${datadir}/postmaster.pid
> - ${rcexec} "${daemon} -D ${datadir} start ${daemon_flags}"
> + ${rcexec} "${daemon} start ${daemon_flags}"
> }
>
> rc_stop() {
> - ${rcexec} "${daemon} -D ${datadir} stop -m fast" || \
> - ${rcexec} "${daemon} -D ${datadir} stop -m immediate"
> + ${rcexec} "${daemon} stop ${daemon_flags} -m fast" || \
> + ${rcexec} "${daemon} stop ${daemon_flags} -m immediate"
> }
>
> rc_cmd $1
> Index: current.html
> ===================================================================
> RCS file: /cvs/www/faq/current.html,v
> retrieving revision 1.1006
> diff -u -p -r1.1006 current.html
> --- current.html 7 Aug 2019 10:34:25 -0000 1.1006
> +++ current.html 17 Aug 2019 20:42:19 -0000
> @@ -152,6 +152,21 @@ Instead disable the route evaluation pro
> <code>rde rib Loc-RIB no evaluate</code>.
>
>
> +<h3 id="r20190817">2019/08/17 - [ports] Datadir added to default postgres
> flags</h3>
> +
> +Default <a href="https://man.openbsd.org/rc.d.8";>rc.d(8)</a> flags
> +for databases/postgresql have been changed from:
> +<pre class="cmdbox">
> +daemon_flags="-w -l /var/postgresql/logfile"
> +</pre>
> +to
> +<pre class="cmdbox">
> +daemon_flags="-D /var/postgresql/data -w -l /var/postgresql/logfile"
> +</pre>
> +if you defined <code>postgresql_flags</code> in
> <code>/etc/rc.conf.local</code>
> +make sure that your entry includes <code>-D /var/postgresql/data</code>.
> +
> +
> <!--
> Two blank lines before new sections.
> New sentences start on new lines.
> Index: rc.d.8
> ===================================================================
> RCS file: /cvs/src/share/man/man8/rc.d.8,v
> retrieving revision 1.34
> diff -u -p -r1.34 rc.d.8
> --- rc.d.8 27 Jul 2017 07:49:05 -0000 1.34
> +++ rc.d.8 17 Aug 2019 20:21:28 -0000
> @@ -137,13 +137,13 @@ with the name of the script.
> For example, postgres is managed through
> .Pa /etc/rc.d/postgresql :
> .Pp
> -.Dl daemon_flags=-w -l /var/postgresql/logfile
> +.Dl daemon_flags="-D /var/postgresql/data -w -l /var/postgresql/logfile"
> .Pp
> To override this and increase the debug log level (keeping the existing
> flags), define the following in
> .Xr rc.conf.local 8 :
> .Pp
> -.Dl postgresql_flags=-w -l /var/postgresql/logfile -d 5
> +.Dl postgresql_flags="-D /var/postgresql/data -w -l /var/postgresql/logfile
> -d 5"
> .Pp
> Each script may define its own defaults, as explained in
> .Xr rc.subr 8 .
--
Antoine
