pg_ctl/miscinit: print "MyStartTime" as a long long instead of long to avoid 2038 problem.

On Tue, Sep 24, 2024 at 07:33:24PM +0000, Max Johnson wrote:

I have found an instance of a time overflow with the start time that is
written in "postmaster.pid". On a 32-bit Linux system, if the system date
is past 01/19/2038, when you start Postgres with `pg_ctl start -D
{datadir} ...`, the start time will have rolled back to approximately
1900. This is an instance of the "2038 problem". On my system, pg_ctl
will not exit if the start time has overflowed.

Nice find. I think this has been the case since the start time was added
to the lock files [0]https://postgr.es/c/30aeda4.

-	snprintf(buffer, sizeof(buffer), "%d\n%s\n%ld\n%d\n%s\n",
+	snprintf(buffer, sizeof(buffer), "%d\n%s\n%lld\n%d\n%s\n",
amPostmaster ? (int) my_pid : -((int) my_pid),
DataDir,
-			 (long) MyStartTime,
+			 (long long) MyStartTime,
PostPortNumber,
socketDir);

I think we should use INT64_FORMAT here. That'll choose the right length
modifier for the platform. And I don't think we need to cast MyStartTime,
since it's a pg_time_t (which is just an int64).

[0]: https://postgr.es/c/30aeda4

--
nathan

Nathan Bossart <nathandbossart@gmail.com> writes:

I think we should use INT64_FORMAT here. That'll choose the right length
modifier for the platform. And I don't think we need to cast MyStartTime,
since it's a pg_time_t (which is just an int64).

Agreed. However, a quick grep finds half a dozen other places that
are casting MyStartTime to long. We should fix them all.

Also note that if any of the other places are using translatable
format strings, INT64_FORMAT is problematic in that context, and
"long long" is a better answer for them.

(I've not dug in the history, but I rather imagine that this is all
a hangover from MyStartTime having once been plain "time_t".)

regards, tom lane

On Tue, Sep 24, 2024 at 04:44:41PM -0400, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

I think we should use INT64_FORMAT here. That'll choose the right length
modifier for the platform. And I don't think we need to cast MyStartTime,
since it's a pg_time_t (which is just an int64).

Agreed. However, a quick grep finds half a dozen other places that
are casting MyStartTime to long. We should fix them all.

+1

Also note that if any of the other places are using translatable
format strings, INT64_FORMAT is problematic in that context, and
"long long" is a better answer for them.

At a glance, I'm not seeing any translatable format strings that involve
MyStartTime. But that is good to know...

--
nathan

Hi there,

I have amended my patch to reflect the changes that were discussed and have verified on my system that it works the same as before. I have also fixed a typo and changed the name of the patch to more accurately reflect what it does now. Please let me know if there is anything else you'd like me to do.

Thanks again,

Max Johnson

Embedded Linux Engineer I

​

NovaTech, LLC

13555 W. 107th Street | Lenexa, KS 66215 ​

O: 913.451.1880

M: 913.742.4580​

novatechautomation.com<http://www.novatechautomation.com/&gt; | NovaTechLinkedIn<https://www.linkedin.com/company/565017&gt;

NovaTech Automation is Net Zero committed. #KeepItCool<https://www.keepitcool.earth/&gt;

Receipt of this email implies compliance with our terms and conditions<https://www.novatechautomation.com/email-terms-conditions&gt;.

________________________________
From: Nathan Bossart <nathandbossart@gmail.com>
Sent: Tuesday, September 24, 2024 3:58 PM
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Max Johnson <max.johnson@novatechautomation.com>; pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: pg_ctl/miscinit: print "MyStartTime" as a long long instead of long to avoid 2038 problem.

On Tue, Sep 24, 2024 at 04:44:41PM -0400, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

I think we should use INT64_FORMAT here. That'll choose the right length
modifier for the platform. And I don't think we need to cast MyStartTime,
since it's a pg_time_t (which is just an int64).

Agreed. However, a quick grep finds half a dozen other places that
are casting MyStartTime to long. We should fix them all.

+1

Also note that if any of the other places are using translatable
format strings, INT64_FORMAT is problematic in that context, and
"long long" is a better answer for them.

At a glance, I'm not seeing any translatable format strings that involve
MyStartTime. But that is good to know...

--
nathan

On Wed, Sep 25, 2024 at 03:17:45PM +0000, Max Johnson wrote:

I have amended my patch to reflect the changes that were discussed and
have verified on my system that it works the same as before. I have also
fixed a typo and changed the name of the patch to more accurately reflect
what it does now. Please let me know if there is anything else you'd like
me to do.

Thanks! I went through all the other uses of MyStartTime and fixed those
as needed, too. Please let me know what you think.

--
nathan

Hi Nathan,

I think your patch looks good, no objections. I am happy to have contributed.

Thanks,
Max
________________________________
From: Nathan Bossart <nathandbossart@gmail.com>
Sent: Wednesday, September 25, 2024 1:48 PM
To: Max Johnson <max.johnson@novatechautomation.com>
Cc: tgl@sss.pgh.pa.us <tgl@sss.pgh.pa.us>; pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: pg_ctl/miscinit: print "MyStartTime" as a long long instead of long to avoid 2038 problem.

On Wed, Sep 25, 2024 at 03:17:45PM +0000, Max Johnson wrote:

I have amended my patch to reflect the changes that were discussed and
have verified on my system that it works the same as before. I have also
fixed a typo and changed the name of the patch to more accurately reflect
what it does now. Please let me know if there is anything else you'd like
me to do.

Thanks! I went through all the other uses of MyStartTime and fixed those
as needed, too. Please let me know what you think.

--
nathan

On Wed, Sep 25, 2024 at 08:04:59PM +0000, Max Johnson wrote:

I think your patch looks good, no objections. I am happy to have contributed.

Great. I've attached what I have staged for commit.

My first instinct was to not bother back-patching this since all
currently-supported versions will have been out of support for over 8 years
by the time this becomes a practical issue. However, I wonder if it makes
sense to back-patch for the kinds of 32-bit embedded systems you cited
upthread. I can imagine that such systems might need to work for a very
long time without any software updates, in which case it'd probably be a
good idea to make this fix available in the next minor release. What do
you think?

--
nathan

I think that it would be a good idea to include these fixes in the next minor release. After working for a couple months on getting our embedded systems 2038 compliant, it has become very apparent that 2038 will be a substantial ordeal. Maximizing the number of systems that include this fix would make things a little easier when that time comes around.

Thanks,

Max
________________________________
From: Nathan Bossart <nathandbossart@gmail.com>
Sent: Thursday, September 26, 2024 9:38 PM
To: Max Johnson <max.johnson@novatechautomation.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>; tgl@sss.pgh.pa.us <tgl@sss.pgh.pa.us>
Subject: Re: pg_ctl/miscinit: print "MyStartTime" as a long long instead of long to avoid 2038 problem.

On Wed, Sep 25, 2024 at 08:04:59PM +0000, Max Johnson wrote:

I think your patch looks good, no objections. I am happy to have contributed.

Great. I've attached what I have staged for commit.

My first instinct was to not bother back-patching this since all
currently-supported versions will have been out of support for over 8 years
by the time this becomes a practical issue. However, I wonder if it makes
sense to back-patch for the kinds of 32-bit embedded systems you cited
upthread. I can imagine that such systems might need to work for a very
long time without any software updates, in which case it'd probably be a
good idea to make this fix available in the next minor release. What do
you think?

--
nathan

On Fri, Sep 27, 2024 at 02:48:01PM +0000, Max Johnson wrote:

I think that it would be a good idea to include these fixes in the next
minor release. After working for a couple months on getting our embedded
systems 2038 compliant, it has become very apparent that 2038 will be a
substantial ordeal. Maximizing the number of systems that include this
fix would make things a little easier when that time comes around.

Alright. I was able to back-patch it to v12 without too much trouble,
fortunately. I'll commit that soon unless anyone else has feedback.

--
nathan

On Fri, Sep 27, 2024 at 02:10:47PM -0500, Nathan Bossart wrote:

Alright. I was able to back-patch it to v12 without too much trouble,
fortunately. I'll commit that soon unless anyone else has feedback.

Committed, thanks!

I refrained from introducing INT64_HEX_FORMAT/UINT64_HEX_FORMAT in c.h
because I felt there was a nonzero chance of that causing problems with
third-party code on the back-branches. We could probably add them for v18,
though.

--
nathan

On Mon, Oct 07, 2024 at 02:00:05PM -0500, Nathan Bossart wrote:

I refrained from introducing INT64_HEX_FORMAT/UINT64_HEX_FORMAT in c.h
because I felt there was a nonzero chance of that causing problems with
third-party code on the back-branches. We could probably add them for v18,
though.

Like so...

--
nathan

On Mon, Oct 07, 2024 at 02:17:06PM -0500, Nathan Bossart wrote:

On Mon, Oct 07, 2024 at 02:00:05PM -0500, Nathan Bossart wrote:

I refrained from introducing INT64_HEX_FORMAT/UINT64_HEX_FORMAT in c.h
because I felt there was a nonzero chance of that causing problems with
third-party code on the back-branches. We could probably add them for v18,
though.

Like so...

If there are no concerns, I plan on committing this soon.

--
nathan

On Tue, Nov 19, 2024 at 05:00:43PM -0600, Nathan Bossart wrote:

If there are no concerns, I plan on committing this soon.

Committed.

--
nathan