[ https://issues.apache.org/jira/browse/HBASE-18085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16018726#comment-16018726
]
Yu Li edited comment on HBASE-18085 at 5/21/17 7:39 AM:
--------------------------------------------------------
bq. As per impl, we should need a boolean state indicating the status of run and which needs
to be volatile?
I thought about using a) volatile boolean, b) AtomicBoolean and c) ReentrantLock#tryLock and
finally chose option c.
Quoting from [stackoverflow|http://stackoverflow.com/questions/3786825/volatile-boolean-vs-atomicboolean]
about volatile boolean v.s. AtomicBoolean:
{noformat}
I use volatile fields when said field is ONLY UPDATED by its owner thread
and the value is only read by other threads
{noformat}
And here each thread might both read and update the field, so AtomicBoolean wins.
And comparing AtomicBoolean and ReentrantLock#tryLock, [this post|http://stackoverflow.com/questions/23473208/atomicboolean-guard-for-not-thread-safe-data-volatile-piggybacking]
suggests the latter one.
Let me know your point if different opinion sir, thanks. [~anoop.hbase]
bq. May be we should take a new approach all together? Than calling purge on every call to
getLock()?
Yeah, I also thought about control purge frequency, but a little bit lost in the counting
standard. Based on counter or timing? And what should the threshold be? Also not sure whether
any regression after such changes, so I chose to limit the change to control the risk.
was (Author: carp84):
bq. As per impl, we should need a boolean state indicating the status of run and which needs
to be volatile?
I thought about using a) volatile boolean, b) AtomicBoolean and c) ReentrantLock#tryLock and
finally chose option c.
Quoting from [stackoverflow|http://stackoverflow.com/questions/3786825/volatile-boolean-vs-atomicboolean]
about volatile boolean v.s. AtomicBoolean:
{noformat}
I use volatile fields when said field is ONLY UPDATED by its owner thread and the value is
only read by other threads
{noformat}
And here each thread might both read and update the field, so AtomicBoolean wins.
And comparing AtomicBoolean and ReentrantLock#tryLock, [this post|http://stackoverflow.com/questions/23473208/atomicboolean-guard-for-not-thread-safe-data-volatile-piggybacking]
suggests the latter one.
Let me know your point if different opinion sir, thanks. [~anoop.hbase]
bq. May be we should take a new approach all together? Than calling purge on every call to
getLock()?
Yeah, I also thought about control purge frequency, but a little bit lost in the counting.
Based on counter or timing? And what should the threshold be? Also not sure whether any regression
after such changes, so I chose to limit the change to control the risk.
> Prevent parallel purge in ObjectPool
> ------------------------------------
>
> Key: HBASE-18085
> URL: https://issues.apache.org/jira/browse/HBASE-18085
> Project: HBase
> Issue Type: Bug
> Reporter: Yu Li
> Assignee: Yu Li
> Attachments: e89l05465.st3.jstack
>
>
> Parallel purge in ObjectPool is meaningless and will cause contention issue since {{ReferenceQueue#poll}}
has synchronization (source code shown below)
> {code}
> public Reference<? extends T> poll() {
> if (head == null)
> return null;
> synchronized (lock) {
> return reallyPoll();
> }
> }
> {code}
> We observed threads blocking on the purge method while using offheap bucket cache, and
we could easily reproduce this by testing the 100% cache hit case in bucket cache with enough
reading threads.
> We propose to add a purgeLock and use tryLock to avoid parallel purge.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)