[
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)
