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

ASF GitHub Bot commented on ACCUMULO-4143:
------------------------------------------

Github user ctubbsii commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/68#discussion_r52676712
  
    --- Diff: 
server/master/src/main/java/org/apache/accumulo/master/Master.java ---
    @@ -1336,7 +1332,11 @@ public void assignedTablet(KeyExtent extent) {
       }
     
       @Override
    -  public Collection<KeyExtent> migrations() {
    -    return migrations.keySet();
    +  public Set<KeyExtent> migrations() {
    +    Set<KeyExtent> migrationsCopy = new HashSet<KeyExtent>();
    +    synchronized (migrations) {
    +      migrationsCopy.addAll(migrations.keySet());
    +    }
    +    return migrationsCopy;
    --- End diff --
    
    I was thinking a bigger risk would be that a user would accidentally 
introduce a RuntimeException (UnsupportedOperationException) because they tried 
to do something which worked against some implementations of the interface, but 
not others, and because wrapped immutability is not something which we can 
check at compile-time. It just seems an unnecessary restriction to me.
    
    It's internal-only code, so we can change it any way we like in the 
future... and I think the risk you're highlighting is lower than the risk of 
somebody accidentally calling an unsupported method... and it's also much 
easier to protect against at the time the implementation changes. If it changes 
from a protective copy to another implementation, I would think it'd be obvious 
for the implementor to check to see if that breaks usage at that time.
    
    For now, I think it makes sense to treat the interface as "snapshot of 
migrations", and the protective copy (vs. another implementation) matches up to 
that interface semantics. We could change the interface method name to 
"migrationsSnapshot()" to be clear, though, and to avoid careless future impl 
changes which change that semantics.


> DeleteTableDuringSplitIT.test() fails occasionally
> --------------------------------------------------
>
>                 Key: ACCUMULO-4143
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-4143
>             Project: Accumulo
>          Issue Type: Bug
>          Components: test
>            Reporter: Christopher Tubbs
>            Assignee: Christopher Tubbs
>             Fix For: 1.6.6, 1.7.1, 1.8.0
>
>
> Saw this twice during 1.6.5-rc1 failing:
> {code}
> test(org.apache.accumulo.test.functional.DeleteTableDuringSplitIT)  Time 
> elapsed: 182.218 sec  <<< ERROR!
> java.util.concurrent.ExecutionException: java.lang.RuntimeException: 
> org.apache.accumulo.core.client.AccumuloException: Internal error processing 
> waitForFateOperation
>       at java.util.concurrent.FutureTask.report(FutureTask.java:122)
>       at java.util.concurrent.FutureTask.get(FutureTask.java:188)
>       at 
> org.apache.accumulo.test.functional.DeleteTableDuringSplitIT.test(DeleteTableDuringSplitIT.java:94)
> Caused by: java.lang.RuntimeException: 
> org.apache.accumulo.core.client.AccumuloException: Internal error processing 
> waitForFateOperation
>       at 
> org.apache.accumulo.test.functional.DeleteTableDuringSplitIT$2.run(DeleteTableDuringSplitIT.java:80)
>       at 
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
>       at java.util.concurrent.FutureTask.run(FutureTask.java:262)
>       at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
>       at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
>       at 
> org.apache.accumulo.trace.instrument.TraceRunnable.run(TraceRunnable.java:47)
>       at 
> org.apache.accumulo.core.util.LoggingRunnable.run(LoggingRunnable.java:34)
>       at java.lang.Thread.run(Thread.java:745)
> Caused by: org.apache.accumulo.core.client.AccumuloException: Internal error 
> processing waitForFateOperation
>       at 
> org.apache.accumulo.core.client.impl.TableOperationsImpl.doFateOperation(TableOperationsImpl.java:336)
>       at 
> org.apache.accumulo.core.client.impl.TableOperationsImpl.doFateOperation(TableOperationsImpl.java:294)
>       at 
> org.apache.accumulo.core.client.impl.TableOperationsImpl.doTableFateOperation(TableOperationsImpl.java:1592)
>       at 
> org.apache.accumulo.core.client.impl.TableOperationsImpl.delete(TableOperationsImpl.java:679)
>       at 
> org.apache.accumulo.test.functional.DeleteTableDuringSplitIT$2.run(DeleteTableDuringSplitIT.java:78)
>       at 
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
>       at java.util.concurrent.FutureTask.run(FutureTask.java:262)
>       at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
>       at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
>       at 
> org.apache.accumulo.trace.instrument.TraceRunnable.run(TraceRunnable.java:47)
>       at 
> org.apache.accumulo.core.util.LoggingRunnable.run(LoggingRunnable.java:34)
>       at java.lang.Thread.run(Thread.java:745)
> Caused by: org.apache.thrift.TApplicationException: Internal error processing 
> waitForFateOperation
>       at 
> org.apache.thrift.TApplicationException.read(TApplicationException.java:111)
>       at org.apache.thrift.TServiceClient.receiveBase(TServiceClient.java:71)
>       at 
> org.apache.accumulo.core.master.thrift.FateService$Client.recv_waitForFateOperation(FateService.java:174)
>       at 
> org.apache.accumulo.core.master.thrift.FateService$Client.waitForFateOperation(FateService.java:159)
>       at 
> org.apache.accumulo.core.client.impl.TableOperationsImpl.waitForFateOperation(TableOperationsImpl.java:266)
>       at 
> org.apache.accumulo.core.client.impl.TableOperationsImpl.doFateOperation(TableOperationsImpl.java:308)
>       at 
> org.apache.accumulo.core.client.impl.TableOperationsImpl.doFateOperation(TableOperationsImpl.java:294)
>       at 
> org.apache.accumulo.core.client.impl.TableOperationsImpl.doTableFateOperation(TableOperationsImpl.java:1592)
>       at 
> org.apache.accumulo.core.client.impl.TableOperationsImpl.delete(TableOperationsImpl.java:679)
>       at 
> org.apache.accumulo.test.functional.DeleteTableDuringSplitIT$2.run(DeleteTableDuringSplitIT.java:78)
>       at 
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
>       at java.util.concurrent.FutureTask.run(FutureTask.java:262)
>       at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
>       at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
>       at 
> org.apache.accumulo.trace.instrument.TraceRunnable.run(TraceRunnable.java:47)
>       at 
> org.apache.accumulo.core.util.LoggingRunnable.run(LoggingRunnable.java:34)
>       at java.lang.Thread.run(Thread.java:745)
> {code}



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

Reply via email to