ctubbsii commented on a change in pull request #1851: URL: https://github.com/apache/accumulo/pull/1851#discussion_r551802157
########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; Review comment: commons-codec is not currently a direct dependency of the test module. I don't think it's worth adding it here, since the use of sha1 will likely trigger the secbugs plugin anyway. I would use Guava's Hasher.sha512 instead. It has a convenient hashString method. ########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; +import org.junit.Test; + +public class ZooMutatorIT extends AccumuloClusterHarness { + /** + * A simple stress test that looks for race conditions in + * {@link ZooReaderWriter#mutateOrCreate(String, byte[], org.apache.accumulo.fate.zookeeper.ZooReaderWriter.Mutator)} + */ + @Test + public void concurrentMutatorTest() throws Exception { + try (var client = Accumulo.newClient().from(getClientProps()).build()) { + + @SuppressWarnings("resource") + var context = (ClientContext) client; + var secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET); + + ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(), + context.getZooKeepersSessionTimeOut(), secret); + + var execServ = Executors.newFixedThreadPool(16); + + String initialData = DigestUtils.sha1Hex("Accumulo Zookeeper Mutator test 1/4/21") + " 0"; + + // This map is used to ensure multiple threads do not successfully write the same value and no + // values are skipped. The hash in the value also verifies similar things in a different way. + ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>(); + + for (int i = 0; i < 16; i++) { + execServ.execute(() -> { + try { + + int count = 0; + while (count < 200) { + byte[] val = + zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue); + var nextCount = getCount(val); + assertTrue(nextCount > count); + count = nextCount; + countCounts.merge(nextCount, 1, Integer::sum); + } + + } catch (Exception e) { + throw new RuntimeException(e); Review comment: RTE won't stop the test from passing. ########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; +import org.junit.Test; + +public class ZooMutatorIT extends AccumuloClusterHarness { Review comment: Since this messes with ZK, it might be best to configure this with the MiniClusterOnlyTests Junit category. ```suggestion @Category(MiniClusterOnlyTests.class) public class ZooMutatorIT extends AccumuloClusterHarness { ``` ########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; +import org.junit.Test; + +public class ZooMutatorIT extends AccumuloClusterHarness { + /** + * A simple stress test that looks for race conditions in + * {@link ZooReaderWriter#mutateOrCreate(String, byte[], org.apache.accumulo.fate.zookeeper.ZooReaderWriter.Mutator)} + */ + @Test + public void concurrentMutatorTest() throws Exception { + try (var client = Accumulo.newClient().from(getClientProps()).build()) { + + @SuppressWarnings("resource") + var context = (ClientContext) client; + var secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET); Review comment: Some of these uses of `var` are a bit questionable, since they don't make the line shorter or more readable, nor are they used when the variable's type is obvious. I would use the actual type in many of these. ```suggestion String secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET); ``` ########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; +import org.junit.Test; + +public class ZooMutatorIT extends AccumuloClusterHarness { + /** + * A simple stress test that looks for race conditions in + * {@link ZooReaderWriter#mutateOrCreate(String, byte[], org.apache.accumulo.fate.zookeeper.ZooReaderWriter.Mutator)} + */ + @Test + public void concurrentMutatorTest() throws Exception { + try (var client = Accumulo.newClient().from(getClientProps()).build()) { + + @SuppressWarnings("resource") + var context = (ClientContext) client; + var secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET); + + ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(), + context.getZooKeepersSessionTimeOut(), secret); + + var execServ = Executors.newFixedThreadPool(16); + + String initialData = DigestUtils.sha1Hex("Accumulo Zookeeper Mutator test 1/4/21") + " 0"; + + // This map is used to ensure multiple threads do not successfully write the same value and no + // values are skipped. The hash in the value also verifies similar things in a different way. + ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>(); + + for (int i = 0; i < 16; i++) { + execServ.execute(() -> { + try { + + int count = 0; + while (count < 200) { + byte[] val = + zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue); + var nextCount = getCount(val); Review comment: ```suggestion int nextCount = getCount(val); ``` ########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; +import org.junit.Test; + +public class ZooMutatorIT extends AccumuloClusterHarness { + /** + * A simple stress test that looks for race conditions in + * {@link ZooReaderWriter#mutateOrCreate(String, byte[], org.apache.accumulo.fate.zookeeper.ZooReaderWriter.Mutator)} + */ + @Test + public void concurrentMutatorTest() throws Exception { + try (var client = Accumulo.newClient().from(getClientProps()).build()) { + + @SuppressWarnings("resource") + var context = (ClientContext) client; Review comment: ```suggestion try (var client = Accumulo.newClient().from(getClientProps()).build(); var context = (ClientContext) client) { ``` ########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; +import org.junit.Test; + +public class ZooMutatorIT extends AccumuloClusterHarness { + /** + * A simple stress test that looks for race conditions in + * {@link ZooReaderWriter#mutateOrCreate(String, byte[], org.apache.accumulo.fate.zookeeper.ZooReaderWriter.Mutator)} + */ + @Test + public void concurrentMutatorTest() throws Exception { + try (var client = Accumulo.newClient().from(getClientProps()).build()) { + + @SuppressWarnings("resource") + var context = (ClientContext) client; + var secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET); + + ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(), + context.getZooKeepersSessionTimeOut(), secret); + + var execServ = Executors.newFixedThreadPool(16); + + String initialData = DigestUtils.sha1Hex("Accumulo Zookeeper Mutator test 1/4/21") + " 0"; + + // This map is used to ensure multiple threads do not successfully write the same value and no + // values are skipped. The hash in the value also verifies similar things in a different way. + ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>(); + + for (int i = 0; i < 16; i++) { + execServ.execute(() -> { + try { + + int count = 0; + while (count < 200) { + byte[] val = + zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue); + var nextCount = getCount(val); + assertTrue(nextCount > count); Review comment: This assertion can fail in threads, but since the executor has no uncaught exception handler, they just cause the threads to die, but the test passes anyway (with a messy stack trace printed to STDERR). ########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; +import org.junit.Test; + +public class ZooMutatorIT extends AccumuloClusterHarness { + /** + * A simple stress test that looks for race conditions in + * {@link ZooReaderWriter#mutateOrCreate(String, byte[], org.apache.accumulo.fate.zookeeper.ZooReaderWriter.Mutator)} + */ + @Test + public void concurrentMutatorTest() throws Exception { + try (var client = Accumulo.newClient().from(getClientProps()).build()) { + + @SuppressWarnings("resource") + var context = (ClientContext) client; + var secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET); + + ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(), + context.getZooKeepersSessionTimeOut(), secret); + + var execServ = Executors.newFixedThreadPool(16); Review comment: `var` is reasonable here, but only if the variable name makes it clear what type it represents. ```suggestion var executor = Executors.newFixedThreadPool(16); ``` ########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; +import org.junit.Test; + +public class ZooMutatorIT extends AccumuloClusterHarness { + /** + * A simple stress test that looks for race conditions in + * {@link ZooReaderWriter#mutateOrCreate(String, byte[], org.apache.accumulo.fate.zookeeper.ZooReaderWriter.Mutator)} + */ + @Test + public void concurrentMutatorTest() throws Exception { + try (var client = Accumulo.newClient().from(getClientProps()).build()) { + + @SuppressWarnings("resource") + var context = (ClientContext) client; + var secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET); + + ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(), + context.getZooKeepersSessionTimeOut(), secret); + + var execServ = Executors.newFixedThreadPool(16); + + String initialData = DigestUtils.sha1Hex("Accumulo Zookeeper Mutator test 1/4/21") + " 0"; + + // This map is used to ensure multiple threads do not successfully write the same value and no + // values are skipped. The hash in the value also verifies similar things in a different way. + ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>(); + + for (int i = 0; i < 16; i++) { + execServ.execute(() -> { + try { + + int count = 0; + while (count < 200) { + byte[] val = + zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue); + var nextCount = getCount(val); + assertTrue(nextCount > count); + count = nextCount; + countCounts.merge(nextCount, 1, Integer::sum); + } + + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + execServ.shutdown(); + + while (!execServ.awaitTermination(1, TimeUnit.SECONDS)) { + + } + + var actual = zk.getData("/test-zm"); Review comment: ```suggestion byte[] actual = zk.getData("/test-zm"); ``` ########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; +import org.junit.Test; + +public class ZooMutatorIT extends AccumuloClusterHarness { + /** + * A simple stress test that looks for race conditions in + * {@link ZooReaderWriter#mutateOrCreate(String, byte[], org.apache.accumulo.fate.zookeeper.ZooReaderWriter.Mutator)} + */ + @Test + public void concurrentMutatorTest() throws Exception { + try (var client = Accumulo.newClient().from(getClientProps()).build()) { + + @SuppressWarnings("resource") + var context = (ClientContext) client; + var secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET); + + ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(), + context.getZooKeepersSessionTimeOut(), secret); + + var execServ = Executors.newFixedThreadPool(16); + + String initialData = DigestUtils.sha1Hex("Accumulo Zookeeper Mutator test 1/4/21") + " 0"; Review comment: A hard-coded date could of `1/4/21` could generate data that is very confusing when run at some point in the future. ########## File path: test/src/main/java/org/apache/accumulo/test/functional/ZooMutatorIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.commons.codec.digest.DigestUtils; +import org.junit.Test; + +public class ZooMutatorIT extends AccumuloClusterHarness { + /** + * A simple stress test that looks for race conditions in + * {@link ZooReaderWriter#mutateOrCreate(String, byte[], org.apache.accumulo.fate.zookeeper.ZooReaderWriter.Mutator)} + */ + @Test + public void concurrentMutatorTest() throws Exception { + try (var client = Accumulo.newClient().from(getClientProps()).build()) { + + @SuppressWarnings("resource") + var context = (ClientContext) client; + var secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET); + + ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(), + context.getZooKeepersSessionTimeOut(), secret); + + var execServ = Executors.newFixedThreadPool(16); + + String initialData = DigestUtils.sha1Hex("Accumulo Zookeeper Mutator test 1/4/21") + " 0"; + + // This map is used to ensure multiple threads do not successfully write the same value and no + // values are skipped. The hash in the value also verifies similar things in a different way. + ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>(); + + for (int i = 0; i < 16; i++) { + execServ.execute(() -> { + try { + + int count = 0; + while (count < 200) { + byte[] val = + zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue); + var nextCount = getCount(val); + assertTrue(nextCount > count); + count = nextCount; + countCounts.merge(nextCount, 1, Integer::sum); + } + + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + execServ.shutdown(); + + while (!execServ.awaitTermination(1, TimeUnit.SECONDS)) { + + } + + var actual = zk.getData("/test-zm"); + int settledCount = getCount(actual); + + assertTrue(settledCount >= 200); + + String expected = initialData; + + for (int i = 1; i <= settledCount; i++) { + assertEquals(1, (int) countCounts.get(i)); + expected = nextValue(expected); + } + + assertEquals(settledCount, countCounts.size()); + assertEquals(expected, new String(actual, UTF_8)); + } + } + + private String nextValue(String currString) { + var tokens = currString.split(" "); + var currHash = tokens[0]; + var count = Integer.parseInt(tokens[1]); Review comment: ```suggestion String[] tokens = currString.split(" "); String currHash = tokens[0]; int count = Integer.parseInt(tokens[1]); ``` ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
