You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by keith-turner <gi...@git.apache.org> on 2017/01/24 21:23:50 UTC

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

GitHub user keith-turner opened a pull request:

    https://github.com/apache/accumulo/pull/206

    ACCUMULO-4575 Fixed concurrent delete table bug

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/keith-turner/accumulo concurrent-deletes

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/accumulo/pull/206.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #206
    
----
commit f4b331e84dce8161ab238930887bd0f753d2bf78
Author: Keith Turner <kt...@apache.org>
Date:   2017-01-24T21:21:10Z

    ACCUMULO-4575 Fixed concurrent delete table bug

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/206#discussion_r97679380
  
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java ---
    @@ -184,4 +188,12 @@ public ClusterUser getUser(int offset) {
         checkArgument(offset >= 0 && offset < users.size(), "Invalid offset, should be non-negative and less than " + users.size());
         return users.get(offset);
       }
    +
    +  @Override
    +  public AccumuloConfiguration getSiteConfiguration() {
    +    Configuration conf = new Configuration(false);
    +    Path accumuloSite = new Path(serverAccumuloConfDir, "accumulo-site.xml");
    --- End diff --
    
    What happens if this file is unreadable (filesystem perms are off) or missing (misconfiguration from human error)? Should this fail hard with an uncheck Exception? Or just log WARN message?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/206#discussion_r97665002
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/functional/ConcurrentDeleteTableIT.java ---
    @@ -0,0 +1,150 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.accumulo.test.functional;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Random;
    +import java.util.TreeSet;
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.ExecutorService;
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.Future;
    +
    +import org.apache.accumulo.core.Constants;
    +import org.apache.accumulo.core.client.AccumuloException;
    +import org.apache.accumulo.core.client.AccumuloSecurityException;
    +import org.apache.accumulo.core.client.BatchWriter;
    +import org.apache.accumulo.core.client.BatchWriterConfig;
    +import org.apache.accumulo.core.client.Connector;
    +import org.apache.accumulo.core.client.Instance;
    +import org.apache.accumulo.core.client.MutationsRejectedException;
    +import org.apache.accumulo.core.client.TableNotFoundException;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.security.Authorizations;
    +import org.apache.accumulo.core.zookeeper.ZooUtil;
    +import org.apache.accumulo.fate.AdminUtil;
    +import org.apache.accumulo.fate.AdminUtil.FateStatus;
    +import org.apache.accumulo.fate.ZooStore;
    +import org.apache.accumulo.fate.zookeeper.IZooReaderWriter;
    +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter;
    +import org.apache.accumulo.harness.AccumuloClusterIT;
    +import org.apache.hadoop.io.Text;
    +import org.apache.zookeeper.KeeperException;
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +public class ConcurrentDeleteTableIT extends AccumuloClusterIT {
    +
    +  private static final String SCHEME = "digest";
    +
    +  private static final String USER = "accumulo";
    +
    +  @Test
    +  public void testConcurrentDeleteTablesOps() throws Exception {
    +    final Connector c = getConnector();
    +    String[] tables = getUniqueNames(2);
    +
    +    TreeSet<Text> splits = new TreeSet<>();
    +
    +    for (int i = 0; i < 1000; i++) {
    +      Text split = new Text(String.format("%09x", i * 100000));
    +      splits.add(split);
    +    }
    +
    +    ExecutorService es = Executors.newFixedThreadPool(20);
    +
    +    int count = 0;
    +    for (final String table : tables) {
    +      c.tableOperations().create(table);
    +      c.tableOperations().addSplits(table, splits);
    +      writeData(c, table);
    +      if (count == 1) {
    +        c.tableOperations().flush(table, null, null, true);
    +      }
    +      count++;
    +
    +      final CountDownLatch cdl = new CountDownLatch(20);
    +
    +      List<Future<?>> futures = new ArrayList<>();
    +
    +      for (int i = 0; i < 20; i++) {
    +        Future<?> future = es.submit(new Runnable() {
    +
    +          @Override
    +          public void run() {
    +            try {
    +              cdl.countDown();
    +              cdl.await();
    +              c.tableOperations().delete(table);
    +            } catch (TableNotFoundException e) {
    +              // expected
    +            } catch (InterruptedException | AccumuloException | AccumuloSecurityException e) {
    +              throw new RuntimeException(e);
    +            }
    +          }
    +        });
    +
    +        futures.add(future);
    +      }
    +
    +      for (Future<?> future : futures) {
    +        future.get();
    +      }
    +
    +      try {
    +        c.createScanner(table, Authorizations.EMPTY);
    +        Assert.fail("Expected table " + table + " to be gone.");
    +      } catch (TableNotFoundException tnfe) {
    +        // expected
    +      }
    +
    +      FateStatus fateStatus = getFateStatus();
    +
    +      // ensure there are no dangling locks... before ACCUMULO-4575 was fixed concurrent delete tables could fail and leave dangling locks.
    +      Assert.assertEquals(0, fateStatus.getDanglingHeldLocks().size());
    +      Assert.assertEquals(0, fateStatus.getDanglingWaitingLocks().size());
    +    }
    +
    +    es.shutdown();
    +  }
    +
    +  private FateStatus getFateStatus() throws KeeperException, InterruptedException {
    +    Instance instance = getConnector().getInstance();
    +    AdminUtil<String> admin = new AdminUtil<>(false);
    +    String secret = "DONTTELL";
    --- End diff --
    
    The right place to get it from would be the `AccumuloCluster` available to you from the `getCluster()` static method on AccumuloClusterIT. Sadly, this is not presently exposed.
    
    I think adding a `AccumuloConfiguration getServerConfiguration();` method onto `AccumuloCluster` is the proper fix.
    
    From there, a `MiniAccumuloClusterImpl` can create an `AccumuloConfiguration` with its `accumulo-site.xml` file and a `StandaloneAccumuloCluster` can create one from its `serverAccumuloConfDir` variable.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/206#discussion_r97679738
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/functional/ConcurrentDeleteTableIT.java ---
    @@ -0,0 +1,147 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.accumulo.test.functional;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Random;
    +import java.util.TreeSet;
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.ExecutorService;
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.Future;
    +
    +import org.apache.accumulo.core.Constants;
    +import org.apache.accumulo.core.client.AccumuloException;
    +import org.apache.accumulo.core.client.AccumuloSecurityException;
    +import org.apache.accumulo.core.client.BatchWriter;
    +import org.apache.accumulo.core.client.BatchWriterConfig;
    +import org.apache.accumulo.core.client.Connector;
    +import org.apache.accumulo.core.client.Instance;
    +import org.apache.accumulo.core.client.MutationsRejectedException;
    +import org.apache.accumulo.core.client.TableNotFoundException;
    +import org.apache.accumulo.core.conf.Property;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.security.Authorizations;
    +import org.apache.accumulo.core.zookeeper.ZooUtil;
    +import org.apache.accumulo.fate.AdminUtil;
    +import org.apache.accumulo.fate.AdminUtil.FateStatus;
    +import org.apache.accumulo.fate.ZooStore;
    +import org.apache.accumulo.fate.zookeeper.IZooReaderWriter;
    +import org.apache.accumulo.harness.AccumuloClusterIT;
    +import org.apache.accumulo.server.zookeeper.ZooReaderWriterFactory;
    +import org.apache.hadoop.io.Text;
    +import org.apache.zookeeper.KeeperException;
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +public class ConcurrentDeleteTableIT extends AccumuloClusterIT {
    +
    +  @Test
    +  public void testConcurrentDeleteTablesOps() throws Exception {
    +    final Connector c = getConnector();
    +    String[] tables = getUniqueNames(2);
    +
    +    TreeSet<Text> splits = new TreeSet<>();
    +
    +    for (int i = 0; i < 1000; i++) {
    +      Text split = new Text(String.format("%09x", i * 100000));
    +      splits.add(split);
    +    }
    +
    +    ExecutorService es = Executors.newFixedThreadPool(20);
    +
    +    int count = 0;
    +    for (final String table : tables) {
    +      c.tableOperations().create(table);
    +      c.tableOperations().addSplits(table, splits);
    +      writeData(c, table);
    +      if (count == 1) {
    +        c.tableOperations().flush(table, null, null, true);
    +      }
    +      count++;
    +
    +      final CountDownLatch cdl = new CountDownLatch(20);
    +
    +      List<Future<?>> futures = new ArrayList<>();
    +
    +      for (int i = 0; i < 20; i++) {
    +        Future<?> future = es.submit(new Runnable() {
    --- End diff --
    
    Did you find this test deterministically reproducing this problem?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner closed the pull request at:

    https://github.com/apache/accumulo/pull/206


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/206#discussion_r97679602
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/functional/BackupMasterIT.java ---
    @@ -39,7 +41,8 @@ public void test() throws Exception {
         // create a backup
         Process backup = exec(Master.class);
         try {
    -      ZooReaderWriter writer = new ZooReaderWriter(cluster.getZooKeepers(), 30 * 1000, "digest", "accumulo:DONTTELL".getBytes());
    +      String secret = getCluster().getSiteConfiguration().get(Property.INSTANCE_SECRET);
    --- End diff --
    
    So much nicer.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/206#discussion_r97662865
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/functional/ConcurrentDeleteTableIT.java ---
    @@ -0,0 +1,150 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.accumulo.test.functional;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Random;
    +import java.util.TreeSet;
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.ExecutorService;
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.Future;
    +
    +import org.apache.accumulo.core.Constants;
    +import org.apache.accumulo.core.client.AccumuloException;
    +import org.apache.accumulo.core.client.AccumuloSecurityException;
    +import org.apache.accumulo.core.client.BatchWriter;
    +import org.apache.accumulo.core.client.BatchWriterConfig;
    +import org.apache.accumulo.core.client.Connector;
    +import org.apache.accumulo.core.client.Instance;
    +import org.apache.accumulo.core.client.MutationsRejectedException;
    +import org.apache.accumulo.core.client.TableNotFoundException;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.security.Authorizations;
    +import org.apache.accumulo.core.zookeeper.ZooUtil;
    +import org.apache.accumulo.fate.AdminUtil;
    +import org.apache.accumulo.fate.AdminUtil.FateStatus;
    +import org.apache.accumulo.fate.ZooStore;
    +import org.apache.accumulo.fate.zookeeper.IZooReaderWriter;
    +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter;
    +import org.apache.accumulo.harness.AccumuloClusterIT;
    +import org.apache.hadoop.io.Text;
    +import org.apache.zookeeper.KeeperException;
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +public class ConcurrentDeleteTableIT extends AccumuloClusterIT {
    +
    +  private static final String SCHEME = "digest";
    +
    +  private static final String USER = "accumulo";
    +
    +  @Test
    +  public void testConcurrentDeleteTablesOps() throws Exception {
    +    final Connector c = getConnector();
    +    String[] tables = getUniqueNames(2);
    +
    +    TreeSet<Text> splits = new TreeSet<>();
    +
    +    for (int i = 0; i < 1000; i++) {
    +      Text split = new Text(String.format("%09x", i * 100000));
    +      splits.add(split);
    +    }
    +
    +    ExecutorService es = Executors.newFixedThreadPool(20);
    +
    +    int count = 0;
    +    for (final String table : tables) {
    +      c.tableOperations().create(table);
    +      c.tableOperations().addSplits(table, splits);
    +      writeData(c, table);
    +      if (count == 1) {
    +        c.tableOperations().flush(table, null, null, true);
    +      }
    +      count++;
    +
    +      final CountDownLatch cdl = new CountDownLatch(20);
    +
    +      List<Future<?>> futures = new ArrayList<>();
    +
    +      for (int i = 0; i < 20; i++) {
    +        Future<?> future = es.submit(new Runnable() {
    +
    +          @Override
    +          public void run() {
    +            try {
    +              cdl.countDown();
    +              cdl.await();
    +              c.tableOperations().delete(table);
    +            } catch (TableNotFoundException e) {
    +              // expected
    +            } catch (InterruptedException | AccumuloException | AccumuloSecurityException e) {
    +              throw new RuntimeException(e);
    +            }
    +          }
    +        });
    +
    +        futures.add(future);
    +      }
    +
    +      for (Future<?> future : futures) {
    +        future.get();
    +      }
    +
    +      try {
    +        c.createScanner(table, Authorizations.EMPTY);
    +        Assert.fail("Expected table " + table + " to be gone.");
    +      } catch (TableNotFoundException tnfe) {
    +        // expected
    +      }
    +
    +      FateStatus fateStatus = getFateStatus();
    +
    +      // ensure there are no dangling locks... before ACCUMULO-4575 was fixed concurrent delete tables could fail and leave dangling locks.
    +      Assert.assertEquals(0, fateStatus.getDanglingHeldLocks().size());
    +      Assert.assertEquals(0, fateStatus.getDanglingWaitingLocks().size());
    +    }
    +
    +    es.shutdown();
    +  }
    +
    +  private FateStatus getFateStatus() throws KeeperException, InterruptedException {
    +    Instance instance = getConnector().getInstance();
    +    AdminUtil<String> admin = new AdminUtil<>(false);
    +    String secret = "DONTTELL";
    --- End diff --
    
    @joshelser  any pointers on how one would obtain the instance secret?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/206#discussion_r97679133
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/functional/ConcurrentDeleteTableIT.java ---
    @@ -0,0 +1,150 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.accumulo.test.functional;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Random;
    +import java.util.TreeSet;
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.ExecutorService;
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.Future;
    +
    +import org.apache.accumulo.core.Constants;
    +import org.apache.accumulo.core.client.AccumuloException;
    +import org.apache.accumulo.core.client.AccumuloSecurityException;
    +import org.apache.accumulo.core.client.BatchWriter;
    +import org.apache.accumulo.core.client.BatchWriterConfig;
    +import org.apache.accumulo.core.client.Connector;
    +import org.apache.accumulo.core.client.Instance;
    +import org.apache.accumulo.core.client.MutationsRejectedException;
    +import org.apache.accumulo.core.client.TableNotFoundException;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.security.Authorizations;
    +import org.apache.accumulo.core.zookeeper.ZooUtil;
    +import org.apache.accumulo.fate.AdminUtil;
    +import org.apache.accumulo.fate.AdminUtil.FateStatus;
    +import org.apache.accumulo.fate.ZooStore;
    +import org.apache.accumulo.fate.zookeeper.IZooReaderWriter;
    +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter;
    +import org.apache.accumulo.harness.AccumuloClusterIT;
    +import org.apache.hadoop.io.Text;
    +import org.apache.zookeeper.KeeperException;
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +public class ConcurrentDeleteTableIT extends AccumuloClusterIT {
    +
    +  private static final String SCHEME = "digest";
    +
    +  private static final String USER = "accumulo";
    +
    +  @Test
    +  public void testConcurrentDeleteTablesOps() throws Exception {
    +    final Connector c = getConnector();
    +    String[] tables = getUniqueNames(2);
    +
    +    TreeSet<Text> splits = new TreeSet<>();
    +
    +    for (int i = 0; i < 1000; i++) {
    +      Text split = new Text(String.format("%09x", i * 100000));
    +      splits.add(split);
    +    }
    +
    +    ExecutorService es = Executors.newFixedThreadPool(20);
    +
    +    int count = 0;
    +    for (final String table : tables) {
    +      c.tableOperations().create(table);
    +      c.tableOperations().addSplits(table, splits);
    +      writeData(c, table);
    +      if (count == 1) {
    +        c.tableOperations().flush(table, null, null, true);
    +      }
    +      count++;
    +
    +      final CountDownLatch cdl = new CountDownLatch(20);
    +
    +      List<Future<?>> futures = new ArrayList<>();
    +
    +      for (int i = 0; i < 20; i++) {
    +        Future<?> future = es.submit(new Runnable() {
    +
    +          @Override
    +          public void run() {
    +            try {
    +              cdl.countDown();
    +              cdl.await();
    +              c.tableOperations().delete(table);
    +            } catch (TableNotFoundException e) {
    +              // expected
    +            } catch (InterruptedException | AccumuloException | AccumuloSecurityException e) {
    +              throw new RuntimeException(e);
    +            }
    +          }
    +        });
    +
    +        futures.add(future);
    +      }
    +
    +      for (Future<?> future : futures) {
    +        future.get();
    +      }
    +
    +      try {
    +        c.createScanner(table, Authorizations.EMPTY);
    +        Assert.fail("Expected table " + table + " to be gone.");
    +      } catch (TableNotFoundException tnfe) {
    +        // expected
    +      }
    +
    +      FateStatus fateStatus = getFateStatus();
    +
    +      // ensure there are no dangling locks... before ACCUMULO-4575 was fixed concurrent delete tables could fail and leave dangling locks.
    +      Assert.assertEquals(0, fateStatus.getDanglingHeldLocks().size());
    +      Assert.assertEquals(0, fateStatus.getDanglingWaitingLocks().size());
    +    }
    +
    +    es.shutdown();
    +  }
    +
    +  private FateStatus getFateStatus() throws KeeperException, InterruptedException {
    +    Instance instance = getConnector().getInstance();
    +    AdminUtil<String> admin = new AdminUtil<>(false);
    +    String secret = "DONTTELL";
    --- End diff --
    
    I added a getSiteConfiguration method


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/206#discussion_r97682477
  
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java ---
    @@ -184,4 +188,12 @@ public ClusterUser getUser(int offset) {
         checkArgument(offset >= 0 && offset < users.size(), "Invalid offset, should be non-negative and less than " + users.size());
         return users.get(offset);
       }
    +
    +  @Override
    +  public AccumuloConfiguration getSiteConfiguration() {
    +    Configuration conf = new Configuration(false);
    +    Path accumuloSite = new Path(serverAccumuloConfDir, "accumulo-site.xml");
    --- End diff --
    
    That would be how I presently swing (fail hard/fast)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/206#discussion_r97682436
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/functional/ConcurrentDeleteTableIT.java ---
    @@ -0,0 +1,147 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.accumulo.test.functional;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Random;
    +import java.util.TreeSet;
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.ExecutorService;
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.Future;
    +
    +import org.apache.accumulo.core.Constants;
    +import org.apache.accumulo.core.client.AccumuloException;
    +import org.apache.accumulo.core.client.AccumuloSecurityException;
    +import org.apache.accumulo.core.client.BatchWriter;
    +import org.apache.accumulo.core.client.BatchWriterConfig;
    +import org.apache.accumulo.core.client.Connector;
    +import org.apache.accumulo.core.client.Instance;
    +import org.apache.accumulo.core.client.MutationsRejectedException;
    +import org.apache.accumulo.core.client.TableNotFoundException;
    +import org.apache.accumulo.core.conf.Property;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.security.Authorizations;
    +import org.apache.accumulo.core.zookeeper.ZooUtil;
    +import org.apache.accumulo.fate.AdminUtil;
    +import org.apache.accumulo.fate.AdminUtil.FateStatus;
    +import org.apache.accumulo.fate.ZooStore;
    +import org.apache.accumulo.fate.zookeeper.IZooReaderWriter;
    +import org.apache.accumulo.harness.AccumuloClusterIT;
    +import org.apache.accumulo.server.zookeeper.ZooReaderWriterFactory;
    +import org.apache.hadoop.io.Text;
    +import org.apache.zookeeper.KeeperException;
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +public class ConcurrentDeleteTableIT extends AccumuloClusterIT {
    +
    +  @Test
    +  public void testConcurrentDeleteTablesOps() throws Exception {
    +    final Connector c = getConnector();
    +    String[] tables = getUniqueNames(2);
    +
    +    TreeSet<Text> splits = new TreeSet<>();
    +
    +    for (int i = 0; i < 1000; i++) {
    +      Text split = new Text(String.format("%09x", i * 100000));
    +      splits.add(split);
    +    }
    +
    +    ExecutorService es = Executors.newFixedThreadPool(20);
    +
    +    int count = 0;
    +    for (final String table : tables) {
    +      c.tableOperations().create(table);
    +      c.tableOperations().addSplits(table, splits);
    +      writeData(c, table);
    +      if (count == 1) {
    +        c.tableOperations().flush(table, null, null, true);
    +      }
    +      count++;
    +
    +      final CountDownLatch cdl = new CountDownLatch(20);
    +
    +      List<Future<?>> futures = new ArrayList<>();
    +
    +      for (int i = 0; i < 20; i++) {
    +        Future<?> future = es.submit(new Runnable() {
    --- End diff --
    
    Cool. That's great.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/206#discussion_r97680645
  
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java ---
    @@ -184,4 +188,12 @@ public ClusterUser getUser(int offset) {
         checkArgument(offset >= 0 && offset < users.size(), "Invalid offset, should be non-negative and less than " + users.size());
         return users.get(offset);
       }
    +
    +  @Override
    +  public AccumuloConfiguration getSiteConfiguration() {
    +    Configuration conf = new Configuration(false);
    +    Path accumuloSite = new Path(serverAccumuloConfDir, "accumulo-site.xml");
    --- End diff --
    
    I Am not sure what the current code will do in that case.  Personally I think it should fail here, instead of later where it make it harder to debug the issue.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/206#discussion_r97680409
  
    --- Diff: test/src/test/java/org/apache/accumulo/test/functional/ConcurrentDeleteTableIT.java ---
    @@ -0,0 +1,147 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.accumulo.test.functional;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Random;
    +import java.util.TreeSet;
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.ExecutorService;
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.Future;
    +
    +import org.apache.accumulo.core.Constants;
    +import org.apache.accumulo.core.client.AccumuloException;
    +import org.apache.accumulo.core.client.AccumuloSecurityException;
    +import org.apache.accumulo.core.client.BatchWriter;
    +import org.apache.accumulo.core.client.BatchWriterConfig;
    +import org.apache.accumulo.core.client.Connector;
    +import org.apache.accumulo.core.client.Instance;
    +import org.apache.accumulo.core.client.MutationsRejectedException;
    +import org.apache.accumulo.core.client.TableNotFoundException;
    +import org.apache.accumulo.core.conf.Property;
    +import org.apache.accumulo.core.data.Mutation;
    +import org.apache.accumulo.core.security.Authorizations;
    +import org.apache.accumulo.core.zookeeper.ZooUtil;
    +import org.apache.accumulo.fate.AdminUtil;
    +import org.apache.accumulo.fate.AdminUtil.FateStatus;
    +import org.apache.accumulo.fate.ZooStore;
    +import org.apache.accumulo.fate.zookeeper.IZooReaderWriter;
    +import org.apache.accumulo.harness.AccumuloClusterIT;
    +import org.apache.accumulo.server.zookeeper.ZooReaderWriterFactory;
    +import org.apache.hadoop.io.Text;
    +import org.apache.zookeeper.KeeperException;
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +public class ConcurrentDeleteTableIT extends AccumuloClusterIT {
    +
    +  @Test
    +  public void testConcurrentDeleteTablesOps() throws Exception {
    +    final Connector c = getConnector();
    +    String[] tables = getUniqueNames(2);
    +
    +    TreeSet<Text> splits = new TreeSet<>();
    +
    +    for (int i = 0; i < 1000; i++) {
    +      Text split = new Text(String.format("%09x", i * 100000));
    +      splits.add(split);
    +    }
    +
    +    ExecutorService es = Executors.newFixedThreadPool(20);
    +
    +    int count = 0;
    +    for (final String table : tables) {
    +      c.tableOperations().create(table);
    +      c.tableOperations().addSplits(table, splits);
    +      writeData(c, table);
    +      if (count == 1) {
    +        c.tableOperations().flush(table, null, null, true);
    +      }
    +      count++;
    +
    +      final CountDownLatch cdl = new CountDownLatch(20);
    +
    +      List<Future<?>> futures = new ArrayList<>();
    +
    +      for (int i = 0; i < 20; i++) {
    +        Future<?> future = es.submit(new Runnable() {
    --- End diff --
    
    Yeah. The handful of times a I ran it before fixing the bug, it failed every time.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo issue #206: ACCUMULO-4575 Fixed concurrent delete table bug

Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:

    https://github.com/apache/accumulo/pull/206
  
    merged in commit df400c5


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---