Bug 47350 - ISO8601DateFormat and AbsoluteTimeDateFormat don't support mulitple timezone
Summary: ISO8601DateFormat and AbsoluteTimeDateFormat don't support mulitple timezone
Status: RESOLVED WONTFIX
Alias: None
Product: Log4j - Now in Jira
Classification: Unclassified
Component: Layout (show other bugs)
Version: 1.2
Hardware: PC Windows XP
: P2 normal
Target Milestone: ---
Assignee: log4j-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-10 13:20 UTC by Gilles Scokart
Modified: 2009-10-10 20:07 UTC (History)
0 users



Attachments
Unit test + fix (6.94 KB, patch)
2009-06-10 13:36 UTC, Gilles Scokart
Details | Diff
Same unit test + patch rebased on the trunk (Revision: 783721) (7.08 KB, patch)
2009-06-11 04:52 UTC, Gilles Scokart
Details | Diff
Code+Unit test fixing the timzone and the thread safety issue (15.00 KB, patch)
2009-06-15 00:25 UTC, Gilles Scokart
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gilles Scokart 2009-06-10 13:20:44 UTC
There is a bug in ISO8601DateFormat and AbsoluteTimeDateFormat when there is appenders that use different timezone.

The effect is that the log of all appenders using ISO8601DateFormat or AbsoluteTimeDateFormat will jump regularily from 1 timezone to an other.

A workaround is to use the SimpleDateFormet (using %d{yyyy-MM-dd HH:mm:ss,SSS} instead of %d for instance).  However, this workaround has a performance impact.
Comment 1 Gilles Scokart 2009-06-10 13:36:43 UTC
Created attachment 23789 [details]
Unit test + fix

Here is a unit test and a fix for this bug.
I didn't find the check box like there is in jira saying that I grant you right for it, but feel free to use it like you want (I have anyway signed an ICLA).
Comment 2 Curt Arnold 2009-06-10 21:00:30 UTC
Will look at this, but will be next week at least.

There were a lot of issues with date formatter that were addressed in the log4j 1.3 development and then back ported into the EnhancedPatternLayout in the extras companion.
Comment 3 Gilles Scokart 2009-06-11 04:52:25 UTC
Created attachment 23793 [details]
Same unit test + patch rebased on the trunk (Revision: 783721)

The first patch was done from the tag.  I update it to align it on the trunk.

I think indeed there is still an issues with those 2 formaters : they are not thread safe.  The access to the static fields are not synchronized.  Because of this you might have jumps of 1 seconds in the past or in the futur.

The problem is that synchronizing the update of the cache might impact the scalability of an application.
Comment 4 Gilles Scokart 2009-06-12 01:01:07 UTC
There is a possibility to make this code thread safe without requiring strong synchronization (by creating a charachter new array every seconds).  I can provide a patch doing this, but that's useless if you plan to bring the EnhancedPatternLayout into the release.
Do you plan to do that, or dis I provide you the thread safe patch?
Comment 5 Gilles Scokart 2009-06-15 00:25:15 UTC
Created attachment 23809 [details]
Code+Unit test fixing the timzone and the thread safety issue

In case you don't want to bring EnhancedPatternLayout into the next release, here is the patch I mentioned last week.  This one should be thread safe (a review is of curse welcome) and fix the original timzone issue I mentioned.
Comment 6 Curt Arnold 2009-10-10 20:07:29 UTC
The patch is substantial enough that I'd prefer to backport EnhancedLayoutPattern from the extras instead of adding that much new code for something that is already fixed there.  I've closed this bug by adding a recommendation in the Javadoc to use EnhancedPatternLayout.

Committed rev 824004.