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

Close

InheritableThreadLocal vs ThreadLocal

Pragnesh
2009-06-24
2013-05-09
  • Pragnesh
    Pragnesh
    2009-06-24

    Hi Jason,

    To do the tracing you are using ThreadLocal. However was there a reason why you are not using InheritableThreadLocal as in a multithreaded web app like portal you would typically have forked portlets and InheritableThreadLocal variables would make more sense in that case.
    Do let me know your thoughts on the same?

    Warm regards
    Pragnesh

     
    • Jason Trump
      Jason Trump
      2009-06-24

      hi pragnesh,

      the threadlocal points to an instance of BehaviorEvent, which is only safe for single-threaded access by design.  we shouldn't use an InheritableThreadLocal here, as it would allow concurrent access.  more to the point, the reason we have a thread-local at all is to monitor execution time at variable stack depths within a single thread of execution.

      it might be interesting in the case of forked renders to allow child events in separate threads to share a parent event (which i think is the case you are talking about).  however, we then need some way to indicate such parallelism in the data.

      jt

       
    • Pragnesh
      Pragnesh
      2009-06-24

      Hi Jason,

      I understand that you are storing the hierarchical BehaviorEvent object as a ThreadLocal object.

      Could you please elaborate your last line on parallelism in the data. Do you mean capturing the parallelism in the data part of Behavior event? If yes then we can just catch ThreadID also as part of the eventData. This way we can associate events to the parent ID and if they are started as a child thread of the original eventID and differentiate using ThreadID.

      I am unable to see a problem in this for using InheritableThreadLocal object. Please let me know if I am missing something. Especially where exactly will this code break.

       
      • Jason Trump
        Jason Trump
        2009-06-24

        BehaviorEvent is not thread-safe.  Specifically the eventData field can easily be corrupted under multi-threaded access.  InheritableThreadLocal is not truly thread-local, it allows multiple threads to access the same object.

        If we did a naive switch to InheritableThreadLocal now, I do not think the existing code would break.  However, custom event interceptors would have multi-threaded access to the parent event.  Child event interceptors could (for example) access the parent event object and attempt to update its event data.

        It is reasonable to assume that people writing their own interceptors will have to obey certain contracts; for example, we could render the parent event immutable while child events are running.

        I think the most important thing to consider is whether tracing asynchronous call trees in the same VM can be met by our earlier discussion on distributed transaction identifiers.  If we can implement with sufficient generality such that both cases are covered (distributed transactions across VMs, asynchronous call trees within the same VM), I would prefer that to an approach that complicates the code for simple single-threaded call trees.

        jt

         
        • Pragnesh
          Pragnesh
          2009-06-25

          Hi Jason,

          How does this idea sound to you:

          create an InheritableThreadLocal and ThreadLocal object in BehaviorTrackingManagerImpl like

          private InheritableThreadLocal rootEvent = new InheritableThreadLocal(); //Noone except the thread creating this should change this.

          private ThreadLocal currentThreadRootEvent = new ThreadLocal();

          when creating the firstEvent (when event is null,currentThreadRootEvent is null and rootEvent is null) in start method assign it all 3.

          When creating the firstEvent in Thread that already has rootEvent assigned in parent Thread(event is null,currentThreadRootEvent is null but rootEvent has a value) pass the parent event as rootEvent. Now in start method we have to change the if condition so that parent can also be the root. The cuurentThreadRootElement and event will be the currentEvent and RootElement will be parent

          in stop method before popping from event stack if currentEvent is same as currentThreadParentEvent make all three variables null (to avoid memory leak of a stale reference for rootEvent)

          Your inputs please

           
          • Pragnesh
            Pragnesh
            2009-06-25

            Hi Jason,

            Also to your point of making InheritableThreadLocal immutable we can just create a wrapper like ImmutableBehaviorEvent. Also I was not clear of the rationale behind to assigning the IDs until we were ready to persist the event. If we just have an ID then we can just assign IDs to the ThreadLocal variable and InheritableThreadLocal variables.

             
            • Pragnesh
              Pragnesh
              2009-06-25

              Also this implementation will help so that while doing Distributed tracing we can just pass the rootEvent ID to the remote Webservice and hence stitch things together.

               
              • Jason Trump
                Jason Trump
                2009-06-27

                now we're getting there :).  Something like a "branch identifier" that can be inherited by either forked threads or remote invocation contexts.

                To summarize, I think we have a couple of new pieces of information to add:

                *  transaction identifier:  relates all operations initiated, directly or indirectly, by a single external event; e.g. user request, web service request, or periodic system process.

                *  thread identifier:  relates all events occurring in a single thread of execution.  as you've suggested we can allow events on child threads to have an inherited parent identifier from a different thread.

                I think we can also follow this theme and add some more useful contextual information

                *  vm identifier:    identifies all events occurring on a single virtual machine
                *  host identifier:  IP or DNS name for the node

                We should also think about how this "distributed processing" support interacts with a more flexible model for selecting which data is captured.

                I would like to give programmers / administrators discrete control over which data elements are selected.  E.g. select which parameters are logged, and possible provide some scheme for controlling the logged parameter's representation by data type.

                More to the point I'd like to consider whether it is possible to detect the deployment type and choose sensible levels of verbosity.  For example, in the single WAR-on-tomcat case it might be nice to suppress some of this new information that is useful in multi-node deployments, since it will only create log noise.

                Finally, I'd like to whether we can incorporate some of this new data into the jmx-persister.

                 
                • Pragnesh
                  Pragnesh
                  2009-06-30

                  Hi Jason,

                  Do you think you would be able to implement this is 1.4.0 RC1?

                   
                  • Jason Trump
                    Jason Trump
                    2009-06-30

                    hi pragnesh,

                    While this is a great feature idea, it is too close to the core of the system to go in so late in a release; it really needs good testing and I expect will go through some significant refinement during implementation.  I'd really like to get 1.4.0 done and publicized, as it's already lagging a bit behind.

                    In any case, once 1.4.0 is out we can plan features for the next release.

                    -jt

                     
    • Pragnesh
      Pragnesh
      2009-06-24

      Hi Jason,

      I like to agree with your idea of a generic implementation. However there is a key difference between new thread and distributed call to a different VM. If a portlet starts a new Thread it is deep inside a framework like lets say many classes inside Portal framework will be creating new child threads. Hence I am not sure of the feasibility of inserting something in case of new Threads. However for remote WS call that is done within our code we have a lot more control.

      Your thoughts on the same please.

      Also would be great to find how kieker is doing the same.

       
    • Jason Trump
      Jason Trump
      2009-06-27

      I checked over there, but it looks like things have been pretty quiet since their initial April release.  I would definitely like to explore exporting data to their analysis engine, though first they have to release their analysis engine to open source.

      It doesn't look like they have a mailing list (?), though they have an issue tracker.  We could make some feature requests and see if we get a response.