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.

Search Discussions

  • Jeevan Chalke at Mar 11, 2010 at 11:02 am
    Oops...
    Forgot to attach the patch.

    Attached here

    Thanks
    On Thu, Mar 11, 2010 at 4:21 PM, Jeevan Chalke 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.
  • Dave Page at Mar 11, 2010 at 11:09 am

    On Thu, Mar 11, 2010 at 10:51 AM, Jeevan Chalke 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)
  • Jeevan Chalke at Mar 11, 2010 at 11:20 am
    Hi Dave,
    On Thu, Mar 11, 2010 at 4:38 PM, Dave Page wrote:

    On Thu, Mar 11, 2010 at 10:51 AM, Jeevan Chalke
    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



    --
    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.
  • Dave Page at Mar 11, 2010 at 11:27 am

    On Thu, Mar 11, 2010 at 11:20 AM, Jeevan Chalke 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?
  • Heikki Linnakangas at Mar 11, 2010 at 12:09 pm

    Dave Page wrote:
    On Thu, Mar 11, 2010 at 10:51 AM, Jeevan Chalke
    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
  • Dave Page at Mar 11, 2010 at 12:26 pm

    On Thu, Mar 11, 2010 at 12:09 PM, Heikki Linnakangas wrote:
    Dave Page wrote:
    On Thu, Mar 11, 2010 at 10:51 AM, Jeevan Chalke
    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.
  • Tom Lane at Mar 11, 2010 at 7:51 pm

    Heikki Linnakangas 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
  • Heikki Linnakangas at Mar 12, 2010 at 7:40 am

    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
  • Tom Lane at Mar 12, 2010 at 7:10 pm

    Heikki Linnakangas 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
  • Tom Lane at Mar 12, 2010 at 7:49 pm

    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
  • Tom Lane at Mar 11, 2010 at 2:59 pm

    Jeevan Chalke 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
  • Jeevan Chalke at Mar 12, 2010 at 9:04 am
    Hi Tom,
    On Thu, Mar 11, 2010 at 8:29 PM, Tom Lane 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.
  • Tom Lane at Mar 12, 2010 at 6:57 pm

    Jeevan Chalke 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
  • Tom Lane at Mar 11, 2010 at 6:56 pm

    Jeevan Chalke 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
  • Robert Haas at Mar 11, 2010 at 7:10 pm

    On Thu, Mar 11, 2010 at 1:49 PM, Tom Lane 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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedMar 11, '10 at 10:57a
activeMar 12, '10 at 7:49p
posts16
users5
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2022 Grokbase