ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Павлухин Иван <vololo...@gmail.com>
Subject Re: Class field ThreadLocal. Why not static?
Date Tue, 11 Sep 2018 10:05:04 GMT
Dmitriy,

Could you point to some piece of code implementing described pattern?

2018-09-11 13:02 GMT+03:00 Павлухин Иван <vololo100@gmail.com>:

> Alex,
>
> ThreadLocal subclass is used in IgniteH2Indexing for simple access to H2
> Connection from current thread. Such subclass has a capability to create
> connection if one does not exist, so obtaining connection is merely
> ThreadLocal.get. Also there are scheduled routines to cleanup connections
> and associated with them statement cache after some expiration time. For
> that reason Map<Thread, H2ConnectionWrapper> is maintained. As query can
> run on user thread we need to cleanup mentioned map to avoid a leak when
> Thread is terminated. So we need to check thread status in cleanup routines
> and remove entries for terminated Threads. And historically there was no
> cleanup for terminated threads and leak was possible. And also great care
> must be taken in order to avoid cyclic reference between ThreadLocal
> instance and a stored value. Which easily could occur if the stored value
> is covered by multiple layers of abstraction.
>
> And I am describing some historical state. Now machinery in IgniteH2Indexing
> is even more complex (I hope we will have a chance to improve it).
>
> 2018-09-11 11:00 GMT+03:00 Alexey Goncharuk <alexey.goncharuk@gmail.com>:
>
>> Ivan,
>>
>> Can you elaborate on the issue with the thread local cleanup you've faced?
>>
>> вт, 11 сент. 2018 г. в 9:13, Павлухин Иван <vololo100@gmail.com>:
>>
>> > Guys,
>> >
>> > As we know ThreadLocal is an instrument which should be used with great
>> > care. And I recently faced with problems related to proper cleanup of
>> > ThreadLocal which is not needed anymore. In my opinion the best thing
>> (in
>> > ideal world) is to get rid of ThreadLocal where possible, but I guess
>> that
>> > it is quite hard (in real world).
>> >
>> > Also, a question comes to my mind. As ThreadLocal is so common in our
>> code,
>> > could you suggest some guidance or code fragments which address proper
>> > ThreadLocal
>> >  lifecycle control and especially cleanup?
>> >
>> > 2018-09-10 12:46 GMT+03:00 Alexey Goncharuk <alexey.goncharuk@gmail.com
>> >:
>> >
>> > > Maxim,
>> > >
>> > > Ignite supports starting multiple instances of Ignite in the same VM,
>> so
>> > > having static thread locals for the fields you mentioned does not
>> work.
>> > >
>> > > Generally, I think thread-local should be bound to the lifespan of the
>> > > component it describes. Static thread-locals are hard to clean-up and
>> > they
>> > > often lead to leaks, so I would rather changed existing static
>> > > thread-locals to be non-static.
>> > >
>> > > --AG
>> > >
>> > > пн, 10 сент. 2018 г. в 11:54, Maxim Muzafarov <maxmuzaf@gmail.com>:
>> > >
>> > > > Igniters,
>> > > >
>> > > > According to javadoc [1] class ThreadLocal:
>> > > > `ThreadLocal instances are typically private *static* fields in
>> classes
>> > > > that wish to associate state with a thread (e.g., a user ID or
>> > > Transaction
>> > > > ID).`
>> > > >
>> > > > So, AFAIK non-static ThreadLocal usage means as `per thread - per
>> class
>> > > > instance`. What the real cases of using non-static ThreadLocal class
>> > > fields
>> > > > in Ignite code project? When we need it?
>> > > >
>> > > > In Ignite code project I've found ThreadLocal usage as:
>> > > >  - non-static - 67
>> > > >  - static  - 68
>> > > >
>> > > > Back to my example, I've checked FileWriteAheadLogManager. It has:
>> > > > 1) private final ThreadLocal<Boolean> interrupted [2]
>> > > > 2) private final ThreadLocal<WALPointer> lastWALPtr [3]
>> > > > I think both of these fields should be set and used as `static`. Can
>> > > anyone
>> > > > confirm it?
>> > > >
>> > > >
>> > > > [1]
>> > https://docs.oracle.com/javase/8/docs/api/java/lang/ThreadLocal.html
>> > > > [2]
>> > > >
>> > > > https://github.com/apache/ignite/blob/master/modules/
>> > > core/src/main/java/org/apache/ignite/internal/processors/
>> > > cache/persistence/wal/FileWriteAheadLogManager.java#L253
>> > > > [3]
>> > > >
>> > > > https://github.com/apache/ignite/blob/master/modules/
>> > > core/src/main/java/org/apache/ignite/internal/processors/
>> > > cache/persistence/wal/FileWriteAheadLogManager.java#L340
>> > > > --
>> > > > --
>> > > > Maxim Muzafarov
>> > > >
>> > >
>> >
>> >
>> >
>> > --
>> > Best regards,
>> > Ivan Pavlukhin
>> >
>>
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>



-- 
Best regards,
Ivan Pavlukhin

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message