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

Eshcar Hillel commented on HBASE-17379:
---------------------------------------

If you think about it the bug was there all the time since we first introduced 
the compacting pipeline. 
Consider a get operation and in-memory compaction or a flush to disk running 
concurrently. The former scans the pipeline while the later changes it. 
HBASE-17081 just caused the to be visible, reverting it won't fix the bug.
[~stack] I agree we should minimize synchronization. But we do need some 
synchronization to avoid ConcurrentModificationException in such cases.

So here is a suggestion -- [~ted_yu] let me know what you think:
Use a read-write lock.
All operations that update the pipeline  /or its version number use the write 
lock.
Only one (new) private method uses the read lock: cloneListForReadOnly(). It 
holds the lock  for a very short period in which it clones the list and returns 
the result.
All public read-only methods like get size and get scanners, first invoke 
cloneListForReadOnly() and then do whatever they need with the cloned list. 
With this solution no concurrent modification exception can occur.
Great.
Now, you might claim that this solution creates a lot of garbage.
To avoid this garbage the method cloneListForReadOnly() can cache the cloned 
list in the CompactionPipeline object and return the same cloned list when 
invoked again. A dirty bit indicates wether the cloned list is valid. Updating 
methods set the dirty bit on (under the write lock) and cloneListForReadOnly() 
set the bit off whenever creating a new clone.

> Lack of synchronization in CompactionPipeline#getScanners()
> -----------------------------------------------------------
>
>                 Key: HBASE-17379
>                 URL: https://issues.apache.org/jira/browse/HBASE-17379
>             Project: HBase
>          Issue Type: Bug
>    Affects Versions: 2.0.0
>            Reporter: Ted Yu
>            Assignee: Ted Yu
>         Attachments: 17379.v1.txt, 17379.v2.txt, 17379.v3.txt, 17379.v4.txt, 
> 17379.v5.txt
>
>
> From 
> https://builds.apache.org/job/PreCommit-HBASE-Build/5053/testReport/org.apache.hadoop.hbase.regionserver/TestHRegionWithInMemoryFlush/testWritesWhileGetting/
>  :
> {code}
> java.io.IOException: java.util.ConcurrentModificationException
>       at 
> org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.handleException(HRegion.java:5886)
>       at 
> org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.initializeScanners(HRegion.java:5856)
>       at 
> org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.<init>(HRegion.java:5819)
>       at 
> org.apache.hadoop.hbase.regionserver.HRegion.instantiateRegionScanner(HRegion.java:2786)
>       at 
> org.apache.hadoop.hbase.regionserver.HRegion.getScanner(HRegion.java:2766)
>       at org.apache.hadoop.hbase.regionserver.HRegion.get(HRegion.java:7036)
>       at org.apache.hadoop.hbase.regionserver.HRegion.get(HRegion.java:7015)
>       at org.apache.hadoop.hbase.regionserver.HRegion.get(HRegion.java:6994)
>       at 
> org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:4141)
>       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:498)
>       at 
> org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
>       at 
> org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
>       at 
> org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
>       at 
> org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
>       at 
> org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
>       at 
> org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
>       at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
>       at org.junit.rules.RunRules.evaluate(RunRules.java:20)
>       at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
>       at 
> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
>       at 
> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
>       at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
>       at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
>       at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
>       at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
>       at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
>       at 
> org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:298)
>       at 
> org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:292)
>       at java.util.concurrent.FutureTask.run(FutureTask.java:266)
>       at java.lang.Thread.run(Thread.java:745)
> Caused by: java.util.ConcurrentModificationException: null
>       at 
> java.util.LinkedList$ListItr.checkForComodification(LinkedList.java:966)
>       at java.util.LinkedList$ListItr.next(LinkedList.java:888)
>       at 
> org.apache.hadoop.hbase.regionserver.CompactionPipeline.getScanners(CompactionPipeline.java:220)
>       at 
> org.apache.hadoop.hbase.regionserver.CompactingMemStore.getScanners(CompactingMemStore.java:298)
>       at 
> org.apache.hadoop.hbase.regionserver.HStore.getScanners(HStore.java:1154)
>       at org.apache.hadoop.hbase.regionserver.Store.getScanners(Store.java:97)
>       at 
> org.apache.hadoop.hbase.regionserver.StoreScanner.getScannersNoCompaction(StoreScanner.java:353)
>       at 
> org.apache.hadoop.hbase.regionserver.StoreScanner.<init>(StoreScanner.java:210)
>       at 
> org.apache.hadoop.hbase.regionserver.HStore.createScanner(HStore.java:1892)
>       at 
> org.apache.hadoop.hbase.regionserver.HStore.getScanner(HStore.java:1880)
>       at 
> org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.initializeScanners(HRegion.java:5842)
>       at 
> org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.<init>(HRegion.java:5819)
>       at 
> org.apache.hadoop.hbase.regionserver.HRegion.instantiateRegionScanner(HRegion.java:2786)
>       at 
> org.apache.hadoop.hbase.regionserver.HRegion.getScanner(HRegion.java:2766)
>       at org.apache.hadoop.hbase.regionserver.HRegion.get(HRegion.java:7036)
> {code}
> The cause is in CompactionPipeline#getScanners() where there is no 
> synchronization around iterating pipeline.
> The code causing ConcurrentModificationException:
> {code}
>     for (Segment segment : this.pipeline) {
> {code}
> was introduced by HBASE-17081



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to