Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#107 Time conversion (TimeZone, DST) (jtds 1.2.5)

v1.2
accepted
momo
None
8
2012-09-22
2010-05-16
Rainer Schwarze
No

Hi,

the attached patch addresses TimeZone/DST issues in the 1.2.5 release of jTDS:
(this is an updated version of this patch for 1.2.2: relates to https://sourceforge.net/tracker/?func=detail&aid=2731952&group_id=33291&atid=407764 )

1) Time conversion with user supplied Calendars:

With default TimeZone "America/New_York", setting Timestamp 2009-03-08 02:00:00 with a UTC Calendar is converted to 2009-03-08 03:00:00 (note 03:00 instead of 02:00). This is because at that time the DST switch occurs and 02:00 in the zone America/New_York doesn't exist (01:59 -> 03:00).

The patch addresses this problem by creating DateTime instances directly upon calls such as PreparedStatement.setTimestamp(int, Timestamp, Calendar) and reading from DateTime instances directly in calls such as ResultSet.getTimestamp(int, Calendar).

Best wishes,
Rainer

PS: The data truncation part, which was included in the 1.2.2 patch, is not included in this 1.2.5 patch.

Discussion

  • Hi,

    I am working on a patch which keeps the Calendar instances in ThreadLocal instead of in the Connection(JDBC2) instance. To make all my tests pass, I had to "clear" the ThreadLocals within ConnectionJDBC2.close(). (Calling ThreadLocal.remove()).

    From the discussion for my 1.2.2 related patch I got the impression, that a ThreadLocal preserving patch would be preferred over Connection instance Calendar. Is that correct? Where is a good place to discuss the issues which probably require to "clear" the ThreadLocal? (forum?)

    Thanks and best wishes,
    Rainer

     
  • Hi,

    almost forgot: I assume a patch for the trunk is welcome, because of the differences between 1.2.5 and trunk.
    I was working on that, but decided to polish my existing 1.2.5 patches first, because I got a lot of new failures when running the unit tests. Can we discuss which of the failures I am getting are normal in the trunk now and which may be due to some potential glitches in my setup? (here in the comments, or a forum thread?)

    Thanks and best wishes,
    Rainer

     
  • momo
    momo
    2010-05-18

    Rainer,

    thank you for your efforts! This is a crucial topic so any help is highly appreciated.

    I applied your patch to my workspace and a number of tests fail for me as well. I didn't have the time to look into that matter in detail and I'm going on vacations today so I will not be able to further investigate what's going wrong within the next 2 weeks. But I agree this may need some further discussion so I'd suggest we simply use this tracker for further talk.

    To answer your questions:

    1. CVS Trunk contains the current jTDS 2 code base, but since there are so many problems with that I'd prefer to limit work to the 1.2.5 branch, at least for the time being. I'm still unsure whether jTDS 2 will ever see the light of day in it's current form or if a complete rewrite will be neccessary.

    2. You are right assuming that a ThreadLocal solution would be preferred, simply because using Connection-bound Calendar objects may become a serious performance bottleneck in highly multi-threaded applications.

    Thanks again,
    momo

     
  • Hi,

    I thought a bit about having the Calendar instance in the Connection versus in a ThreadLocal: (I didn't have to use ThreadLocal very much in the past, so if I came to wrong conclusions, please correct me)

    To make my test cases run well when using ThreadLocal, I needed to clear ThreadLocal instances when closing Connections. Otherwise subsequent runs picked up old Calendar instances from ThreadLocals which where now invalid because a different default TimeZone was set in the meantime. So I assume, when using ThreadLocal we need to cleanup in the end to provide predictable behaviour.

    Cleaning up doesn't seem to be that easy: ThreadLocal gets the map from a Thread instance. So without knowing all the threads which may have triggered a ThreadLocal.get() we can't be sure, that we cleanup all the Calendar instances. To be sure, we need to track the Calendar instances, which are created in ThreadLocal.initialValue() and this in one way or another ends up in a list of Calendar instances bound to a Connection. Another issue pops up: When a thread uses two connections, don't these Connections share the same Calendar instance via ThreadLocal?

    There is an issue short named "forced timezone" ( https://sourceforge.net/tracker/?func=detail&aid=1848272&group_id=33291&atid=407765 ) in one of the earlier comments which requires to set the timezone in the Calendar instance. If this issue shall be resolved (which has to be asked, if the solution collides with other issues), a Connection bound Calendar instance seems to be nice. I'm not yet sure, whether this can be solved with ThreadLocals or not. (two Connections sharing a Calendar instance?)

    Connection bound Calendars require synchronization which becomes a blocking bottle neck in some use cases.

    How about giving Statements and ResultSets their own Calendar instance which is "cloned" from the Connection's Calendar upon creation of Statements and ResultSets?

    Now the question is, whether to synchronize on the Statement's Calendar instances. The FAQ says this:

    "Can jTDS be used in a multi threaded application?

    As a general principle we try and keep synchronization to a minimum both for performance and deadlock reasons. The only part of jTDS we guarantee is thread safe is the Connection object, and multi threaded access to Statements is discouraged (except for issuing cancels). ..."

    The Statement's Calendar instance can be synchronized and I'd expect that the blocking bottle neck shouldn't be an issue here. Depending on the interpretation of "discouraged", even the synchronization could be spared.

    If no synchronization on the Statement's Calendar instances is needed, the code wouldn't be too complicated. Methods like setDate(int, Date) internally use the Statement's Calendar and setDate(int, Date, Calendar) uses the given Calendar instances without accessing the Statement's Calendar.

    What do you think?

    Best wishes,
    Rainer

     
  • Hi,

    I uploaded another patch (jtfs_1.2.5_dst_b2.diff) which goes on top of the first one. This includes the changes to have Calendar instances in Statement, ResultSet etc. instances so they don't have to synchronize on the Connection's Calendar. ThreadLocals are not used any more with this patch.

    This is rather experimental. My unit tests pass, but I did not yet run performance tests.

    Best wishes,
    Rainer

     
  • momo
    momo
    2010-06-30

    Rainer,

    thanks a lot for providing the patches - and sorry for the late reply. I applied your patches to my local working copy and started looking through your code. So far, everything looks great. I'll have to take a closer look at some of your changes and try to run some tests (including performance tests) to prove everything is working as intended. I think I'll be able to include your changes in the next release.

    Thanks again,
    momo

     
  • Hi,

    thanks for the information. If you have any questions, don't hesitate to contact me.

    Best wishes,
    Rainer

     
  • watches is one of the important factors in our daily life.Original watches are always very expensive compared the replica watches.so that is why there are many people like the replica watches,especially the top quality grade replica watches.http://www.bwo888.com is a very professional top quality swiss grade replica watches supplier who can sell many very fashion and classic replica watches.,including top quality and most popular,classic and professional watches brands,such as Rolex,Patek Philippe, Omega,Audemars Piguet,Breguet,Breitling, IWC,Cartier,Panerai,Tag Heuer,Hublot, Chopard,Montblanc,and so on.For our profession and long-term reputation in this filed and market.

    http://www.bwo888.com

     
  • momo
    momo
    2012-09-22

    • status: open --> accepted
    • assigned_to: momo
    • milestone: --> v1.2
     


Anonymous


Cancel   Add attachments