Horton
Menu

Thread

  1. Server crash with older tzload library

    Jeevan Chalke <jeevan.chalke@enterprisedb.com> — 2010-03-11T10:51:37Z

    Hi Tom,

While setting timezone using SET command (say GMT+3:30), postgres sometimes
crashes randomly.
After debugging into the code, it is observed that if tzload() call fails in
pg_tzset() for whatever reason, the returned value of the function then have
garbage values for state variable. And this will assigned to
session_timezone in assign_timezone() function later.

Now as session_timezone.state variable has garbage values, it is causing a
server crash further. Unfortunately it is happening with few garbage values
and not crashing the server always.

Here are the two statements used:

SET TimeZone = 'GMT+3:30';
SELECT '1969-12-31 20:30:00'::timestamptz;

After, initializing tzstate variable to zero in pg_tzset() function, server
crash didn't come up again.

The upstream zoneinfo project just released a new tzcode version, 2010c.
After syncing the code to this version does not lead to server crash. The
new release is now initializing the tzstate variable with zeros to avoid any
garbage values.

PFA, patch which will bring us up-to date to 2010c.

Note: This behavior was observed on Windows machine.

Thanks

-- 
Jeevan B Chalke
Software Engineer, R&D
EnterpriseDB Corporation
The Enterprise Postgres Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are not
the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.

  2. Re: Server crash with older tzload library

    Jeevan Chalke <jeevan.chalke@enterprisedb.com> — 2010-03-11T11:02:13Z

    Oops...
Forgot to attach the patch.

Attached here

Thanks

On Thu, Mar 11, 2010 at 4:21 PM, Jeevan Chalke <
jeevan.chalke@enterprisedb.com> wrote:

> Hi Tom,
>
> While setting timezone using SET command (say GMT+3:30), postgres sometimes
> crashes randomly.
> After debugging into the code, it is observed that if tzload() call fails
> in pg_tzset() for whatever reason, the returned value of the function then
> have garbage values for state variable. And this will assigned to
> session_timezone in assign_timezone() function later.
>
> Now as session_timezone.state variable has garbage values, it is causing a
> server crash further. Unfortunately it is happening with few garbage values
> and not crashing the server always.
>
> Here are the two statements used:
>
> SET TimeZone = 'GMT+3:30';
> SELECT '1969-12-31 20:30:00'::timestamptz;
>
> After, initializing tzstate variable to zero in pg_tzset() function, server
> crash didn't come up again.
>
> The upstream zoneinfo project just released a new tzcode version, 2010c.
> After syncing the code to this version does not lead to server crash. The
> new release is now initializing the tzstate variable with zeros to avoid any
> garbage values.
>
> PFA, patch which will bring us up-to date to 2010c.
>
> Note: This behavior was observed on Windows machine.
>
> Thanks
>
> --
> Jeevan B Chalke
> Software Engineer, R&D
> EnterpriseDB Corporation
> The Enterprise Postgres Company
>
> Phone: +91 20 30589500
>
> Website: www.enterprisedb.com
> EnterpriseDB Blog: http://blogs.enterprisedb.com/
> Follow us on Twitter: http://www.twitter.com/enterprisedb
>
> This e-mail message (and any attachment) is intended for the use of the
> individual or entity to whom it is addressed. This message contains
> information from EnterpriseDB Corporation that may be privileged,
> confidential, or exempt from disclosure under applicable law. If you are not
> the intended recipient or authorized to receive this for the intended
> recipient, any use, dissemination, distribution, retention, archiving, or
> copying of this communication is strictly prohibited. If you have received
> this e-mail in error, please notify the sender immediately by reply e-mail
> and delete this message.
>



-- 
Jeevan B Chalke
Software Engineer, R&D
EnterpriseDB Corporation
The Enterprise Postgres Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are not
the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.

  3. Re: Server crash with older tzload library

    Dave Page <dpage@pgadmin.org> — 2010-03-11T11:08:59Z

    On Thu, Mar 11, 2010 at 10:51 AM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:

> PFA, patch which will bring us up-to date to 2010c.

Hi Jeevan,

We're already up to 2010e in CVS:
http://archives.postgresql.org/pgsql-committers/2010-03/msg00131.php
(not 2010d as the commit message mistakenly states)


-- 
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
PG East Conference: http://www.enterprisedb.com/community/nav-pg-east-2010.do

  4. Re: Server crash with older tzload library

    Jeevan Chalke <jeevan.chalke@enterprisedb.com> — 2010-03-11T11:20:26Z

    Hi Dave,

On Thu, Mar 11, 2010 at 4:38 PM, Dave Page <dpage@pgadmin.org> wrote:

> On Thu, Mar 11, 2010 at 10:51 AM, Jeevan Chalke
> <jeevan.chalke@enterprisedb.com> wrote:
>
> > PFA, patch which will bring us up-to date to 2010c.
>
> Hi Jeevan,
>
> We're already up to 2010e in CVS:
> http://archives.postgresql.org/pgsql-committers/2010-03/msg00131.php
> (not 2010d as the commit message mistakenly states)
>
> Ohh, kewl.

BTW, I am using git repository and there I didn't see any changes on master
branch.
Is it possible to sync git with cvs?

Thanks


> --
> Dave Page
> EnterpriseDB UK: http://www.enterprisedb.com
> PG East Conference:
> http://www.enterprisedb.com/community/nav-pg-east-2010.do
>



-- 
Jeevan B Chalke
Software Engineer, R&D
EnterpriseDB Corporation
The Enterprise Postgres Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are not
the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.

  5. Re: Server crash with older tzload library

    Dave Page <dpage@pgadmin.org> — 2010-03-11T11:27:12Z

    On Thu, Mar 11, 2010 at 11:20 AM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:

> BTW, I am using git repository and there I didn't see any changes on master
> branch.
> Is it possible to sync git with cvs?

Hmm, that should happen automagically within a few minutes of a commit
to CVS I thought. Magnus?


-- 
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
PG East Conference: http://www.enterprisedb.com/community/nav-pg-east-2010.do

  6. Re: Server crash with older tzload library

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-03-11T12:09:17Z

    Dave Page wrote:
> On Thu, Mar 11, 2010 at 10:51 AM, Jeevan Chalke
> <jeevan.chalke@enterprisedb.com> wrote:
> 
>> PFA, patch which will bring us up-to date to 2010c.
> 
> Hi Jeevan,
> 
> We're already up to 2010e in CVS:
> http://archives.postgresql.org/pgsql-committers/2010-03/msg00131.php
> (not 2010d as the commit message mistakenly states)

No, Jeevan is talking about tzcode, not tzdata. The zoneinfo library is
split into two parts, we update the data part at each release, but we
don't sync up our code with upstream code changes regularly.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

  7. Re: Server crash with older tzload library

    Dave Page <dpage@pgadmin.org> — 2010-03-11T12:26:02Z

    On Thu, Mar 11, 2010 at 12:09 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> Dave Page wrote:
>> On Thu, Mar 11, 2010 at 10:51 AM, Jeevan Chalke
>> <jeevan.chalke@enterprisedb.com> wrote:
>>
>>> PFA, patch which will bring us up-to date to 2010c.
>>
>> Hi Jeevan,
>>
>> We're already up to 2010e in CVS:
>> http://archives.postgresql.org/pgsql-committers/2010-03/msg00131.php
>> (not 2010d as the commit message mistakenly states)
>
> No, Jeevan is talking about tzcode, not tzdata. The zoneinfo library is
> split into two parts, we update the data part at each release, but we
> don't sync up our code with upstream code changes regularly.

Ah, OK. Sorry for the noise.


-- 
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
PG East Conference: http://www.enterprisedb.com/community/nav-pg-east-2010.do

  8. Re: Server crash with older tzload library

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-03-11T14:59:41Z

    Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
> While setting timezone using SET command (say GMT+3:30), postgres sometimes
> crashes randomly.

I can't reproduce that:

regression=# SET TimeZone = 'GMT+3:30';
SET
regression=# SELECT '1969-12-31 20:30:00'::timestamptz;
        timestamptz        
---------------------------
 1969-12-31 20:30:00-03:30
(1 row)


			regards, tom lane

  9. Re: Server crash with older tzload library

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-03-11T18:49:15Z

    Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
> The upstream zoneinfo project just released a new tzcode version, 2010c.
> After syncing the code to this version does not lead to server crash. The
> new release is now initializing the tzstate variable with zeros to avoid any
> garbage values.

> PFA, patch which will bring us up-to date to 2010c.

I've applied the update to 2010c since that apparently fixes some
misbehaviors in obscure time zones (where is America/Godthab???).
However, the proposed addition of explicit clears of the tzstate
struct doesn't match any upstream change that I can see.  I inserted
explicit initializations to random data instead and still couldn't
provoke a crash.  While it seems harmless enough to explicitly zero
it, I'd like to see an instance of the reported crash, because I have
a feeling that the real problem you're dealing with is elsewhere.
If you can't provoke it reliably, maybe the zeroing didn't really
fix it.

			regards, tom lane

  10. Re: Server crash with older tzload library

    Robert Haas <robertmhaas@gmail.com> — 2010-03-11T19:10:16Z

    On Thu, Mar 11, 2010 at 1:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
>> The upstream zoneinfo project just released a new tzcode version, 2010c.
>> After syncing the code to this version does not lead to server crash. The
>> new release is now initializing the tzstate variable with zeros to avoid any
>> garbage values.
>
>> PFA, patch which will bring us up-to date to 2010c.
>
> I've applied the update to 2010c since that apparently fixes some
> misbehaviors in obscure time zones (where is America/Godthab???).

Greenland, apparently.

http://www.travelmath.com/time-zone/America/Godthab

...Robert

  11. Re: Server crash with older tzload library

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-03-11T19:51:09Z

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> No, Jeevan is talking about tzcode, not tzdata. The zoneinfo library is
> split into two parts, we update the data part at each release, but we
> don't sync up our code with upstream code changes regularly.

It strikes me that maybe we are putting ourselves at risk by blithely
pushing tzdata updates into back branches without also pushing tzcode
updates.  However, doing this would mean updating the back branches for
64bit tzdata, which is not a small change.  Heikki, do you remember how
much that patch affected stuff outside the tzcode files proper?

In any case it would be madness to try to get that into the releases due
to be wrapped today, but maybe it should be on the to-do list to make it
happen soon (in particular, before we desupport 8.0, because this could
matter for the long-term usability of 8.0).

			regards, tom lane

  12. Re: Server crash with older tzload library

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-03-12T07:40:31Z

    Tom Lane wrote:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
>> No, Jeevan is talking about tzcode, not tzdata. The zoneinfo library is
>> split into two parts, we update the data part at each release, but we
>> don't sync up our code with upstream code changes regularly.
> 
> It strikes me that maybe we are putting ourselves at risk by blithely
> pushing tzdata updates into back branches without also pushing tzcode
> updates.

I believe they're designed to be compatible both ways, I remember that
the 64-bit changes in particular were done in such a way that new tzdata
files work with older tzcode versions. I don't know if anyone else is
testing various combinations, though, so it probably would be good to
update tzcode anyway.

>  However, doing this would mean updating the back branches for
> 64bit tzdata, which is not a small change.  Heikki, do you remember how
> much that patch affected stuff outside the tzcode files proper?

There was no changes outside tzcode files.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

  13. Re: Server crash with older tzload library

    Jeevan Chalke <jeevan.chalke@enterprisedb.com> — 2010-03-12T09:04:28Z

    Hi Tom,

On Thu, Mar 11, 2010 at 8:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
> > While setting timezone using SET command (say GMT+3:30), postgres
> sometimes
> > crashes randomly.
>
> I can't reproduce that:
>
> regression=# SET TimeZone = 'GMT+3:30';
> SET
> regression=# SELECT '1969-12-31 20:30:00'::timestamptz;
>        timestamptz
> ---------------------------
>  1969-12-31 20:30:00-03:30
> (1 row)
>
>
Even we were fail to re-produce it always. It is crashing randomly. After
debugging, we found following values in tzstate variable when it crashed. So
I manually modified it at the beginning of pg_tzset() function.

/* Initialize tzstate with some arbitrary values */
tzstate.goback = 277946440;
tzstate.goahead = 0;
tzstate.ats[0] = 8892;

Also not that, tzstate variable remains uninitialized properly when tzload()
call returns -1. To mimic this, I have just renamed posixrules timezone file
to something else. So that tzload returns -1. And as tzload can return -1 in
any failure case and as we don't have that check at callee side tzstate
variable remains uninitialized which leading to a server crash. As above
garbage values too leads to a crash.

In summary, following are the steps to re-produce:
 - Add above three lines at the beginning of the pg_tzset() function
 - make install
 - mv install/share/postgresql/timezone/posixrules
install/share/postgresql/timezoneposixrules_a  (or remove it)
 - start the server
 - run these two statements on psql prompt
   + SET TimeZone = 'GMT+3:30';
   + SELECT '1969-12-31 20:30:00'::timestamptz;

BTW, after your commit, tzload() function now sets goback and goahead
variable to FALSE which fixes the server crash. MemSet in 2010c is just
doing it explicitly before calling tzload though.

Thanks for committing the part of the patch which stopped crashing the
server with above mentioned steps.

Thanks


>                        regards, tom lane
>



-- 
Jeevan B Chalke
Software Engineer, R&D
EnterpriseDB Corporation
The Enterprise Postgres Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are not
the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.

  14. Re: Server crash with older tzload library

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-03-12T18:57:02Z

    Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
> In summary, following are the steps to re-produce:
>  - Add above three lines at the beginning of the pg_tzset() function
>  - make install
>  - mv install/share/postgresql/timezone/posixrules
> install/share/postgresql/timezoneposixrules_a  (or remove it)
>  - start the server
>  - run these two statements on psql prompt
>    + SET TimeZone = 'GMT+3:30';
>    + SELECT '1969-12-31 20:30:00'::timestamptz;

> BTW, after your commit, tzload() function now sets goback and goahead
> variable to FALSE which fixes the server crash. MemSet in 2010c is just
> doing it explicitly before calling tzload though.

Mph.  I still can't reproduce a crash even after doing all that and
reverting the goback/goahead change.  However, it seems to me that that
last indicates that this is the same problem discussed on the upstream
tz mailing list in early February, and their fix was to move the
goback/goahead initialization, ie they fixed it between 2010a and 2010c.
So I think we're good (except that this is another reason why we'd be
well advised to back-port tzcode2010c into our older branches).

If we were to insert a memset I think that pg_tzset is the wrong place
for it anyway.  If tzload or tzparse returns 0, then pg_tzset is entitled
to assume that those functions have set up valid tzstate structure
contents, so it should be on their head to zero the struct if that were
needed.  This was in fact proposed upstream (cf ado's message of 16 Feb
2010 17:33:28) but they eventually chose to just clear the
goback/goahead fields there.  I feel no need to diverge from their
solution.

			regards, tom lane

  15. Re: Server crash with older tzload library

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-03-12T19:10:00Z

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> Tom Lane wrote:
>> It strikes me that maybe we are putting ourselves at risk by blithely
>> pushing tzdata updates into back branches without also pushing tzcode
>> updates.

> I believe they're designed to be compatible both ways, I remember that
> the 64-bit changes in particular were done in such a way that new tzdata
> files work with older tzcode versions. I don't know if anyone else is
> testing various combinations, though, so it probably would be good to
> update tzcode anyway.

I wouldn't really expect a massive failure, but I am worried about the
possibility of misbehavior in corner-case timezones, such as those with
a very large number of DST rule changes.  Also there is the risk of
unfixed bugs in the tzcode functions themselves.  It's not clear to me
for example whether the crash bug Jeevan's been on about was introduced
in the 64bit tzcode changes or is a pre-existing problem.

>> However, doing this would mean updating the back branches for
>> 64bit tzdata, which is not a small change.  Heikki, do you remember how
>> much that patch affected stuff outside the tzcode files proper?

> There was no changes outside tzcode files.

OK, I'm going to double-check that and then back-patch the current
tzcode files.  It's too late for the upcoming releases but probably
it's just as well for this to sit awhile in CVS before we ship it.
We'll at least get some buildfarm cycles on it...

			regards, tom lane

  16. Re: Server crash with older tzload library

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-03-12T19:48:55Z

    I wrote:
> OK, I'm going to double-check that and then back-patch the current
> tzcode files.

Oh, wait, belay that.  If we do that, we will be retroactively breaking
the no-working-64-bit-integer-type support in the older branches.
Probably not a good thing to do in minor releases.  I guess we'll just
have to wait and see if any problems get reported.  I can't see going
to the effort of making the new tzcode stuff behave gracefully without
int64.

			regards, tom lane