Thread: systemd service files vs. postgresql9x-setup

Hi,

Looks like postgresql9[45]-setup is careless when someone tries to use them
in customized
systemd environments (e.g. via drop-in configurations[1]). Currently we do
this:

# this parsing technique fails for PGDATA pathnames containing spaces,
# but there's not much I can do about it given systemctl's output format...
PGDATA=`systemctl show -p Environment "${SERVICE_NAME}.service" |
                sed 's/^Environment=//' | tr ' ' '\n' |
                sed -n 's/^PGDATA=//p' | tail -n 1`

[...some more code later...]

# Get data directory from the service file
PGDATA=`sed -n 's/Environment=PGDATA=//p' "${SERVICE_FILE}"`

So we obviously overwrite any PGDATA setting returned by 'systemctl show'
earlier. If someone uses service files only, this doesn't heavily matter,
since SERVICE_FILE is tested against multiple locations. However, drop-in
configurations are ignored/overwritten with this method. I don't
understand, why we do the sed approach anyways, since 'systemctl show'
already covers all cases, afaics. So i suggest to get rid of handling the
SERVICE_FILE directly and leave 'systemctl show' alone, patch attached.

Opinions?

[1] <http://www.freedesktop.org/software/systemd/man/systemd.unit.html>

--
Thanks

    Bernd

>
> On Aug 12, 2015, at 7:18 AM, Bernd Helmle <mailings@oopsware.de> wrote:
>
> Hi,
>
> Looks like postgresql9[45]-setup is careless when someone tries to use them
> in customized
> systemd environments (e.g. via drop-in configurations[1]). Currently we do
> this:
>
> # this parsing technique fails for PGDATA pathnames containing spaces,
> # but there's not much I can do about it given systemctl's output format...
> PGDATA=`systemctl show -p Environment "${SERVICE_NAME}.service" |
>                sed 's/^Environment=//' | tr ' ' '\n' |
>                sed -n 's/^PGDATA=//p' | tail -n 1`
>
> [...some more code later...]
>
> # Get data directory from the service file
> PGDATA=`sed -n 's/Environment=PGDATA=//p' "${SERVICE_FILE}"`
>
> So we obviously overwrite any PGDATA setting returned by 'systemctl show'
> earlier. If someone uses service files only, this doesn't heavily matter,
> since SERVICE_FILE is tested against multiple locations. However, drop-in
> configurations are ignored/overwritten with this method. I don't
> understand, why we do the sed approach anyways, since 'systemctl show'
> already covers all cases, afaics. So i suggest to get rid of handling the
> SERVICE_FILE directly and leave 'systemctl show' alone, patch attached.
>
> Opinions?
>
> [1] <http://www.freedesktop.org/software/systemd/man/systemd.unit.html>

Unfortunately, I’m not that familiar with systemd yet and Devrim is out for a few weeks, so we might not get to this
veryquickly. 

--On 12. August 2015 16:54:41 -0700 Jeff Frost <jeff@pgexperts.com> wrote:

> Unfortunately, I’m not that familiar with systemd yet and Devrim is out
> for a few weeks, so we might not get to this very quickly.

No worries.

--
Thanks

    Bernd

--On 12. August 2015 16:54:41 -0700 Jeff Frost <jeff@pgexperts.com> wrote:

>> Opinions?
>>
>> [1] <http://www.freedesktop.org/software/systemd/man/systemd.unit.html>
>
> Unfortunately, I’m not that familiar with systemd yet and Devrim is out
> for a few weeks, so we might not get to this very quickly.

So i'd like to step in again into this patch.

We're working with a RHEL7 environment where those drop-in configurations
are heavily used to move PGDATA to different places and i'd like to
preserve that behavior.

Recap:

postgresql9[45]-setup happily overwrites any PGDATA setting derived from
drop-in configurations by examining the service file directly and ignoring
the previous retrieved setting from systemctl.

So this looks worth to fix for any setups, even when other ways to setup
PGDATA are used.

--
Thanks

    Bernd

Hi Bernd,

First of all: Thanks for the patch!

The next set of new minor releases are due in a few weeks. I think we
can apply this patch then.

(We need a issue tracker!)


Regards, Devrim
On Thu, 2015-09-17 at 14:49 +0200, Bernd Helmle wrote:
>
> --On 12. August 2015 16:54:41 -0700 Jeff Frost <jeff@pgexperts.com>
> wrote:
>
> > > Opinions?
> > >
> > > [1] <
> > > http://www.freedesktop.org/software/systemd/man/systemd.unit.html
> > > >
> >
> > Unfortunately, I’m not that familiar with systemd yet and Devrim is
> > out
> > for a few weeks, so we might not get to this very quickly.
>
> So i'd like to step in again into this patch.
>
> We're working with a RHEL7 environment where those drop-in
> configurations
> are heavily used to move PGDATA to different places and i'd like to
> preserve that behavior.
>
> Recap:
>
> postgresql9[45]-setup happily overwrites any PGDATA setting derived
> from
> drop-in configurations by examining the service file directly and
> ignoring
> the previous retrieved setting from systemctl.
>
> So this looks worth to fix for any setups, even when other ways to
> setup
> PGDATA are used.
>
> --
> Thanks
>
>     Bernd
>
>

--
Devrim GÜNDÜZ
Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR

--On 18. September 2015 16:48:09 +0300 Devrim GÜNDÜZ <devrim@gunduz.org>
wrote:

> First of all: Thanks for the patch!
>
> The next set of new minor releases are due in a few weeks. I think we
> can apply this patch then.
>
> (We need a issue tracker!)

Hmm, i've totally forgotten that. I see that it's not committed yet, so i
just go forward and ask if there's something missing we need to address?

--
Thanks

    Bernd