Hi Ivan, For example, I remember thread local buffer in our old WAL implementation: org.apache.ignite.internal.processors.cache.persistence.wal.FsyncModeFileWriteAheadLogManager#tlb
Sincerely, Dmitriy Pavlov вт, 11 сент. 2018 г. в 13:05, Павлухин Иван <[email protected]>: > Dmitriy, > > Could you point to some piece of code implementing described pattern? > > 2018-09-11 13:02 GMT+03:00 Павлухин Иван <[email protected]>: > > > 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 <[email protected] > >: > > > >> Ivan, > >> > >> Can you elaborate on the issue with the thread local cleanup you've > faced? > >> > >> вт, 11 сент. 2018 г. в 9:13, Павлухин Иван <[email protected]>: > >> > >> > 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 < > [email protected] > >> >: > >> > > >> > > 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 <[email protected]>: > >> > > > >> > > > 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 >
