[
https://issues.apache.org/jira/browse/CALCITE-3325?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16924280#comment-16924280
]
Josh Elser commented on CALCITE-3325:
-------------------------------------
[~m2je], I don't believe your problem statement is correct.
First off, the ThreadLocal should not be static. ProtobufTranslationImpl is
already a threadsafe singleton. There's no benefit in making it the ThreadLocal
static because we should only ever have one instance of it per Connection.
The design here is that multiple threads in your application which are
interacting with the same Avatica Connection would be sharing this class. The
ThreadLocal ensures that each Thread which is using your Connection would also
keep using the same buffer to serialize/deserialize Protobuf messages
going/coming to/from the wire.
If you check out the ThreadLocal javadoc, they say this:
{quote}after a thread goes away, all of its copies of thread-local instances
are subject to garbage collection (unless other references to these copies
exist).
{quote}
There are also other instances of UnsynchronizedBuffer's which are used in the
client. Your screen shot is not clear enough to tell me anything. At best, it's
showing me ~256KB of heap usage (30 instances at 8KB) which is next to nothing.
If you're hoping for me to tell you what's going wrong, at a minimum, you
should be sharing the code you are using to test, verbatim instructions on how
to use that code, and the hprof file(s) you generated from your client
application.
Thanks.
> Thread Local Buffer Variable (threadLocalBuffer) In ProtobufTranslationImpl
> Is Defined As Non Static Causing Memory Leak
> ------------------------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-3325
> URL: https://issues.apache.org/jira/browse/CALCITE-3325
> Project: Calcite
> Issue Type: Bug
> Reporter: Mehdi Salarkia
> Priority: Major
> Attachments: Screen Shot 2019-09-05 at 5.18.19 PM.png
>
>
> As we were load testing our system on Apache Phoenix via the thin client
> which uses Avatica we ran into Garbage collection problems. After some
> investigation we could see there are a lot of unreferenced object due to this
> variable:
> {code:java}
> private final ThreadLocal<UnsynchronizedBuffer> threadLocalBuffer =
> new ThreadLocal<UnsynchronizedBuffer>() {
> @Override protected UnsynchronizedBuffer initialValue() {
> return new UnsynchronizedBuffer();
> }
> };
> {code}
> Which seems to be a reusable buffer for serializing/deserializing the data.
> From my understating there is a copy of this variable created per each
> instance of ProtobufTranslationImpl. However the proper use of ThreadLocal it
> seems to be one instance per thread and the current implementation seems to
> be missing the `static` keyword when defining the thread local variable:
> See [https://docs.oracle.com/javase/7/docs/api/java/lang/ThreadLocal.html]
> {code:java}
> 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).
> {code}
> See the attached snapshot from a memory dump we took.
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
