darold / pgbadger

A fast PostgreSQL Log Analyzer
http://pgbadger.darold.net/
PostgreSQL License
3.49k stars 349 forks source link

MS Windows - "--retention" option not working because POSIX::strftime("%s") not supported on MS Windows #775

Closed bbourgier closed 1 year ago

bbourgier commented 1 year ago

Hi,

Environment: MS Windows Server 2016 Strawberry Perl v5.32.1 pgBadger v11.8

On MS Windows implementation (is it perl issue or pure windows issue, I'm not sure), the "%s" tag for POSIX::strftime is not supported.

See test:

C:\pgbadger\__test_retention_11.8.BBO.01>perl -MPOSIX -e "$d=26; $m=04; $y=2023; print POSIX::strftime('%s', 1,1,1,$d,$m-1,$y-1900)"

C:\pgbadger\__test_retention_11.8.BBO.01>

As you can see, the "%s" call returns nothing.

This compatibility issue breaks the "--retention" option as POSIX::strftime('%s' ...) is used to find out which files/folders should be deleted by comparing this epoch time value with the specified "--retention" parameter value.

I searched a bit and worked out a fix which does not use POSIX::strftime('%s' ...).

So here is the fix I suggest.

Basically replace occurrences of:

my $seconds_from_epoch = POSIX::strftime("%s", 1,1,1,$d,$m-1,$y-1900);

With:

my $seconds_from_epoch = timelocal(1,1,1,$d,$m-1,$y-1900);

I did tests in different timezones and the result is the same with POSIX way and with timelocal way. You can see test script used in online interpreter in attachement: readme.bbo.onlinetest.txt

The result in different timezones is:

############# TEST 01
### Timezone: Europe/London

curTime_in_timezone=[Thu Apr 27 08:30:11 2023]

*** POSIX/strftime Method - Seconds From Epoch Computation
seconds_from_epoch_POSIX_strftime=[1682467261]

*** timelocal Method - Seconds From Epoch Computation
seconds_from_epoch_TIMELOCAL_with_timezone=[1682467261]
timezone_offset_in_seconds=[-3600]
seconds_from_epoch_TIMELOCAL_no_timezone=[1682470861]

############# TEST 02
### Timezone: America/Los_Angeles

curTime_in_timezone=[Thu Apr 27 00:30:11 2023]

*** POSIX/strftime Method - Seconds From Epoch Computation
seconds_from_epoch_POSIX_strftime=[1682496061]

*** timelocal Method - Seconds From Epoch Computation
seconds_from_epoch_TIMELOCAL_with_timezone=[1682496061]
timezone_offset_in_seconds=[25200]
seconds_from_epoch_TIMELOCAL_no_timezone=[1682470861]

Looks like the "POSIX::strftime("%s"..." call returns a result in the current timezone and NOT in universal time.

In the pgBadger code, here is the fix I suggest then.

Replace simply the current code ("POSIX::strftime("%s"..." calls) :

                    else
                    {
                        # Remove obsolete days when we are in binary mode
                        # with noreport - there's no week-N directory
                        my $diff_day = $retention * 7 * 86400;
                        my $oldday = POSIX::strftime("%s", 1,1,1,$d,$m-1,$y-1900);
                        my $lastday = $oldday;
                        if (($saved_last_line{datetime} =~ /^(\d+)\-(\d+)\-(\d+) /) ||
                            ($pgb_saved_last_line{datetime} =~ /^(\d+)\-(\d+)\-(\d+) /)) {
                            $lastday = POSIX::strftime("%s", 1,1,1,$3,$2-1,$1-1900);
                        }
                        if (($lastday - $oldday) > $diff_day) {
                            &logmsg('DEBUG', "Removing obsolete directory $outdir/$y/$m/$d");
                            &cleanup_directory("$outdir/$y/$m/$d", 1);
                            push(@rmdays, $d);
                        }
                    }

With:

                    else
                    {
                        # Remove obsolete days when we are in binary mode
                        # with noreport - there's no week-N directory
                        my $diff_day = $retention * 7 * 86400;

                        ###my $oldday = POSIX::strftime("%s", 1,1,1,$d,$m-1,$y-1900);
                        my $oldday = timelocal(1,1,1,$d,$m-1,$y-1900);

                        my $lastday = $oldday;
                        if (($saved_last_line{datetime} =~ /^(\d+)\-(\d+)\-(\d+) /) ||
                            ($pgb_saved_last_line{datetime} =~ /^(\d+)\-(\d+)\-(\d+) /)) {

                            ###$lastday = POSIX::strftime("%s", 1,1,1,$3,$2-1,$1-1900);
                            $lastday = timelocal(1,1,1,$3,$2-1,$1-1900);
                        }
                        if (($lastday - $oldday) > $diff_day) {
                            &logmsg('DEBUG', "Removing obsolete directory $outdir/$y/$m/$d");
                            &cleanup_directory("$outdir/$y/$m/$d", 1);
                            push(@rmdays, $d);
                        }
                    }

I tested it and it works. The files/folders gets deleted when the retention period in weeks is exceeded.

Thanks in advance.

Bertrand

darold commented 1 year ago

Commit cf9f511 applies your patch.