hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eshcar Hillel (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-17434) New Synchronization Scheme for Compaction Pipeline
Date Tue, 10 Jan 2017 08:27:58 GMT

    [ https://issues.apache.org/jira/browse/HBASE-17434?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15814318#comment-15814318

Eshcar Hillel commented on HBASE-17434:

Thanks. So here are my answers (some are already in RB):
Every object o has a lock associated with it; synchronized(o) { .. } block simply takes the
lock on o at the beginning of the lock and release it at the end of the block.
We need a lock to make sure the modification to the attributes of the compaction pipeline
are atomic, and also if we read more than one attribute like in getVersionedList() where we
read both the version number and the readOnlyCopy and we want to have a constant view of them.
It doesn't matter which lock of which object we use, but *how* we use it.
At some point I though to use  synchronized(this) instead of synchronized(pipeline), but I
changed my mind since the lock associated with this is exposed outside of the compaction pipeline,
and we want to make sure no one else contends on the lock we use inside this class.

I moved the increment of the version number from to swap() (it does not appear twice). The
reason was so we have the three steps
apply changes (swap)
inc version
all close to each other.
This way it is easy to see they are inside a synchronized block.
Also, their new order better fits the model, since now when reading a greater version than
expected (inside/outside the lock) it is clear that the suffix was already swapped.

Volatile - I follow the oracle documentation in https://docs.oracle.com/javase/tutorial/essential/concurrency/atomic.html
which says reads and writes of reference variable are always atomic, while long and double
are atomic if declared volatile. Therefor, version which is long needs to be volatile, but
readOnlyCopy is ok without a volatile.

I'm attaching a new patch which changes the pipeline linked list to be final.

> New Synchronization Scheme for Compaction Pipeline
> --------------------------------------------------
>                 Key: HBASE-17434
>                 URL: https://issues.apache.org/jira/browse/HBASE-17434
>             Project: HBase
>          Issue Type: Bug
>            Reporter: Eshcar Hillel
>            Assignee: Eshcar Hillel
>         Attachments: HBASE-17434-V01.patch, HBASE-17434-V02.patch, HBASE-17434-V03.patch,
> A new copyOnWrite synchronization scheme is introduced for the compaction pipeline.
> The new scheme is better since it removes the lock from getSegments() which is invoked
in every get and scan operation, and it reduces the number of LinkedList objects that are
created at runtime, thus can reduce GC (not by much, but still...).
> In addition, it fixes the method getTailSize() in compaction pipeline. This method creates
a MemstoreSize object which comprises the data size and the overhead size of the segment and
needs to be atomic.

This message was sent by Atlassian JIRA

View raw message