0

we have been using ThreadLocal so far to carry some data so as to not clutter the API. However below are some of issues of using thread local that which I dont like

1) over the years the data items being carried in thread local has increased 2) Since we started using threads (for some light weight processing), we have also migrating these data to the threads in the pool and copying them back again

I am thinking of using an in memory DB for these (we doesnt want to add this to the API). I wondering if this approach is good. What r the pros and cons.

ok here is a simple scenario

  1. user logs in and submits a request
  2. system establishes context for this entire request which include - unique id for this request - username - system looged in (user can log into multiple systems) - some DOMAIN EVENTS for later use
  3. the request passes through multiple logical layers (presentation, business domain, rules, integration) etc
  4. in the integration layer, we borrow few threads from pool to parallel pull data from multiple partners. each of the pulls need some data stored earlier in thread local, so we migrate those to the pooled threads
  5. after all data is received from partners, we migrate back the new thread local data accumulated in the child threads to the main thread
  6. at the end of the interaction we persist the DOMAIN events to DB
Nick Bastin
  • 30,415
  • 7
  • 59
  • 78
Aravind Yarram
  • 78,777
  • 46
  • 231
  • 327
  • 7
    I am concerned about your use of ThreadLocal. Are you concerned about concurrency issues? If you aren't, then why are you using ThreadLocal? If you are, then would you plan on having an in-memory database for each running Thread? – Jeremy Mar 24 '10 at 02:30
  • The data items are unique to a given request and we didnt want to pass around that data all over... We generate a unique key for each transaction (or request) and i can centrally store the data (currently in threadlocal) against the unique key – Aravind Yarram Mar 24 '10 at 02:48
  • It's difficult to answer this question right now because it's not clear what the use case is.. You might want to add some illustrative code. – Enno Shioji Mar 24 '10 at 02:48
  • I mean, the `key` in `ThreadLocal` is the `Thread` itself, right? Then you have another unique key?? As for "passing around data", normally one encapsulates whatever data and passes around the reference to it.. Doesn't seem to have anything to do with cluttering the API. I for one am pretty confused.. – Enno Shioji Mar 24 '10 at 02:51
  • storing lots of data in ThreadLocals to avoid "cluttering" an API is a code smell - you're basically passing hidden data to methods and other segments of your code. Avoid doing using ThreadLocals for non-concurrent things – matt b Mar 24 '10 at 02:54
  • @Zwei - Even if we encapsulate all the data in some object say (MyContext) the still all the methods that need a reference to this object still either need to be passed into the method call or set as a property on all the objects that need them right? – Aravind Yarram Mar 24 '10 at 03:05
  • @matt - this might or might not be the code smell...cluttering the API just to pass around this data might also be a code smell and that is exactly why i am looking for other options.... – Aravind Yarram Mar 24 '10 at 03:07
  • No that would not be an indication of bad code. It would be an indication of needing to pass parameters to methods that need them. Passing parameters hidden in Threadlocals is an indication of bad design. – matt b Mar 24 '10 at 03:18
  • @matt - do u sprinkle the corss-cutting concerns all over the code or use aspects...i think the usage here is analogous to that. for e.g. hibernate stores its session in thread local isnt it? – Aravind Yarram Mar 24 '10 at 03:21
  • @Pangea: Your design is already complex enough, so switching it over to a database is just adding more complexity. If I were you I would look into why I need all this data in the ThreadLocals. You said yourself this concept has gotten carried away. It is much safer to pass the appropriate data down through your logic than it is to hide it where anybody can mess with it. Also, unless the data you're storing is Thread-safe, there's no guarantee that it wont be corrupted or unintentionally modified. I'm not saying dump all your ThreadLocals, I'm just saying maybe some data should go elsewhere. – Jeremy Mar 24 '10 at 12:19
  • @Jer - appreciate ur suggestions..but can u tell me why storing this data in in-memory db is bad? Also, I know that carrying data in threadLocals is bad but this decision is taken by the architect and he is adamant that he is not going to add that to the api (unfortunately) – Aravind Yarram Mar 24 '10 at 14:17
  • @Pangea: You didn't answer my question about how the database would be used. Would it be a singularity or would there be an instance per thread/request as you have with the ThreadLocals? – Jeremy Mar 24 '10 at 14:29
  • it would be singular. i dont see a need for thread specific inmemory DB. my idea is to store the data items in the inmemory db against the unique key created per each request and at the end of the each request clear it out from the db... – Aravind Yarram Mar 24 '10 at 14:36
  • 2
    @Pangea: I think an in-memory database is overkill for this. Especially if the data is temporary. But it's tough to tell because I really don't know the type of data being stored. I would suggest you look into an alternative storage solution. Maybe something like a ConcurrentHashMap in a central location. This way if you ever do find a way to fix this design flaw then it is easy to remove. – Jeremy Mar 24 '10 at 14:49
  • @Jer - thx for the suggestion. I am also looking at MapMaker (has eviction policies based on expiry time) as an alternative to in memeory db. Another reason why we dont want to add these data items to the API is becaz most of them are using only during QA/UAT but not in production...so i dont think it is a nice idea to change the API just to enable testing...what do u think? – Aravind Yarram Mar 24 '10 at 15:12
  • @Pangrea: That's probably your best bet then. Overall your use of ThreadLocal isn't very safe and I would try to eliminate them unless you have thread-specific concurrency concerns that makes them useful. I still question the need for storing data like that for QA testing, but that's an entirely different topic. – Jeremy Mar 24 '10 at 15:29

1 Answers1

1

you may want to introduce a request context: http://www.corej2eepatterns.com/Patterns2ndEd/ContextObject.htm

you can handle creation/destruction of such an object in a Filter if you're using a WebContainer or an Interceptor if you're using an ApplicationServer.

cproinger
  • 2,258
  • 1
  • 17
  • 33