MDEV-19436: Fix logrotate problem with twice configured pid-file option

[odenbach]

Hi,

if the pid-file option is configured more than once (e.g. multiple times in different files), my_print_defaults prints it twice, resulting in the logrotate postrotate script failing because of a syntax error. Debian fixed this already (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=830976#42).

Perhaps you could implement this small change in the other branches as well?

Thanks,

Christopher

[an3l]

Hi @odenbach,

Thanks for your contribution.

Similar to other open source projects, the MariaDB Foundation needs to have shared ownership of all code that is included in the MariaDB distribution. The easiest way to achieve this is by submitting your code under the BSD-new license.

The other alternative is to sign the code contribution agreement which can be found here: https://mariadb.com/kb/en/mariadb/mca/

Please indicate in a comment below, or update your PR comment that you are contributing your new code of the whole pull request, including one or several files that are either new files or modified ones, under the BSD-new license or that you have filled out the contribution agreement and sent it.

Thanks,
Anel

[odenbach]

BSD-new is fine.

[an3l]

@fauust can you please take a look?

[grooverdan]

Actually the real problem is checking there is a pidfile. There may not be one existing with systemd and mariadb-10.1+ but mariadb may still be running. You be better just running mysqladmin and ignoring the socket not found error (exit status 1).

[odenbach]

Hi,

I just switched over to using mysqladmin ping instead of looking for pid files. This really seems to make much more sense. I tried it with the server running and without, looked good.

Cheers,

Christopher

[grooverdan]

Good alternative. I like it. +1 from me.

[odenbach]

Hi,

should I provide anything else? The checks above obviously failed for some totally other reason.

Thanks,

Christopher

[vuvova]

I'd simply remove the whole check and just run

mysqladmin flush-engine-log flush-general-log flush-slow-log 2>&1 >/dev/null || :

if mysqladmin ping will fail to connect then mysqladmin flush can fail just the same, no need to run it twice.

[vuvova]

Do you want to fix it yourself or you'd prefer me to do this simple change?

[odenbach]

If you just drop any error messages you will not get noticed if the flush-log command fails for any other reason (e.g. file system full). I would rather prefer my version: Making sure the server can listen to commands and then telling it what to do.

[obendev]

Any news on this? I think it's a good solution.

[CLAassistant]

All committers have signed the CLA.

[ottok]

Another alternative is https://salsa.debian.org/mariadb-team/mariadb-10.4/-/commit/143569f09de024e7e3c2fc82074012e0cbbcd207 but I'd vote for merging this one in.

[ottok]

Note that there is also a support-files/mysql-log-rotate.sh which in CMake build is added to debian/tmp/etc/logrotate.d but not installed. I am considering using it in #1498 so we in future have just one single logrotate script across all distros.

[grooverdan]

if you do, @localstatedir@ (currently defined to datadir in support-files/CMakeLists.txt) in support-files/mysql-log-rotate.sh should be changed to something like @logdir@ and cmake/install_layout.cmake to include INSTALL_LOGDIR_{DEB,RPM,STANDALONE,SRV4}.

support-files/CMakeLists.txt will need SET(logdir ${INSTALL_LOGDIRABS} instead of previous localstatedir.

[ottok]

While this PR is correct, the whole logrotate thing needed an overhaul, so I filed https://jira.mariadb.org/browse/MDEV-22659 and made #1556 which if merged would obsolete this.

[grooverdan]

Thanks @obendev - finally merged for older versions.