I'll explain this. There is not an MDC.getCopy...here's the comparison again:
AsyncAppender calls loggingEvent.getMDCCopy()
Old behavior: - (in loggingEvent.getMDCCopy) check if mdcCopyLookupRequired - if required, Hashtable ht = (Hashtable) MDC.getContext(); - mdcCopy = (Hashtable) ht.clone();
New behavior: - (in loggingEvent.getMDCCopy) check if mdcCopyLookupRequired - if required, MDCContainer context = (MDCContainer) MDC.getContext(); - mdcCopy = context.copy(); [which is simply: return (MDCContainer) clone(); ]
Note that the BasicMDCContainer extends Hashtable, so the only overhead is the copy method calling clone as opposed to calling clone directly (so whatever a single method call overhead is).
Without this fix or something like it, any mutable content in MDC will be carried across threads and will be subject to ConcurrentModificationExceptions at the least and the logging of incorrect context data since the context can change between the time the event was issued and the asychronous append.
How could ConcurrentModificationExceptions occur? Log4j components do not modify objects stored in the MDC. They but do not write... Whatama-missing-here?
Also, just to explain how this all works, if I would have been using *my* MDCContainer, the copy method would not simply call clone...it can do the deep copy since it knows the type of the content.
Thanks for everyone's time on this one.
-C
-----Original Message----- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] Sent: Saturday, January 31, 2004 5:09 AM To: Log4J Developers List Subject: RE: MDC copies
Hi Christian,
I appreciate your explanation. However, my concern is more about the overhead when calling MDC.getCopy() and not the extra overhead in MDC.put. Does that clarify my point?
At 04:42 PM 1/30/2004 -0500, [EMAIL PROTECTED] wrote: >None of the changes I have made should add any serious performance >overhead. > >Old behavior: >MDC.put("name", "value"); (get is similar) >- get thread local map >- get value from tlm >- if value is null > - new Hashtable > - put new hashtable in tlm >- hashtable.put("name", "value"); > >New behavior: >- get thread local map >- get value from tlm >- if value is null > - getFactory (singleton) > - factory.create (just calls new BasicMDCContainer which is just a >subclass of hashtable) > - put new container in tlm >- container.put("name", "value"); > >(Logger is not affected) > >No other code is run unless you use async appender, and that just calls
>clone on the container (delegates to the parent hashtable) by default. > > >I think maybe my explanation was unclear. This is a very low overhead >change. Should I just send it for folks to look at?
If you feel that actual code will make the discussion easier, then please do.
-- Ceki G�lc�
For log4j documentation consider "The complete log4j manual" ISBN: 2970036908 http://www.qos.ch/shop/products/clm_t.jsp
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
-- Ceki G�lc�
For log4j documentation consider "The complete log4j manual"
ISBN: 2970036908 http://www.qos.ch/shop/products/clm_t.jsp
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
