diff --git a/core/pom.xml b/core/pom.xml index 78945cc5dd2..0ab562c1ab6 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -47,6 +47,10 @@ auto-service true + + com.google.code.findbugs + jsr305 + com.google.code.gson gson diff --git a/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java b/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java index 9266a53e510..39632375688 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java +++ b/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java @@ -28,6 +28,8 @@ import java.util.Set; import java.util.SortedSet; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.IteratorSetting; import org.apache.accumulo.core.client.Scanner; import org.apache.accumulo.core.client.TableNotFoundException; @@ -76,6 +78,7 @@ import com.google.common.base.Preconditions; +@NotThreadSafe class RFileScanner extends ScannerOptions implements Scanner { private static class RFileScannerEnvironmentImpl extends ClientServiceEnvironmentImpl { diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerOptions.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerOptions.java index 2b044f6fd84..fb288a519f5 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerOptions.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerOptions.java @@ -35,6 +35,8 @@ import java.util.TreeSet; import java.util.concurrent.TimeUnit; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.IteratorSetting; import org.apache.accumulo.core.client.ScannerBase; import org.apache.accumulo.core.client.sample.SamplerConfiguration; @@ -46,6 +48,7 @@ import org.apache.accumulo.core.util.TextUtil; import org.apache.hadoop.io.Text; +@NotThreadSafe public class ScannerOptions implements ScannerBase { protected List serverSideIteratorList = Collections.emptyList(); diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java index bf47678b9a7..232ff8e8e1e 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java @@ -45,6 +45,8 @@ import java.util.function.Supplier; import java.util.stream.Collectors; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.AccumuloException; import org.apache.accumulo.core.client.AccumuloSecurityException; import org.apache.accumulo.core.client.SampleNotPresentException; @@ -355,6 +357,7 @@ private String getTableInfo() { return context.getPrintableTableInfoFromId(tableId); } + @NotThreadSafe private class QueryTask implements Runnable { private final String tsLocation; diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportPool.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportPool.java index 75db7bffeda..73a8aa0c0bc 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportPool.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportPool.java @@ -41,6 +41,8 @@ import java.util.function.LongSupplier; import java.util.function.Supplier; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.rpc.ThriftUtil; import org.apache.accumulo.core.rpc.clients.ThriftClientTypes; import org.apache.accumulo.core.util.HostAndPort; @@ -577,6 +579,7 @@ public TransportPoolShutdownException(String msg) { private static final long serialVersionUID = 1L; } + @NotThreadSafe private static class CachedTTransport extends TTransport { private final ThriftTransportKey cacheKey; diff --git a/core/src/main/java/org/apache/accumulo/core/crypto/streams/BlockedInputStream.java b/core/src/main/java/org/apache/accumulo/core/crypto/streams/BlockedInputStream.java index 3e37f6f4045..4777b341532 100644 --- a/core/src/main/java/org/apache/accumulo/core/crypto/streams/BlockedInputStream.java +++ b/core/src/main/java/org/apache/accumulo/core/crypto/streams/BlockedInputStream.java @@ -23,10 +23,13 @@ import java.io.IOException; import java.io.InputStream; +import javax.annotation.concurrent.NotThreadSafe; + /** * Reader corresponding to BlockedOutputStream. Expects all data to be in the form of size (int) * data (size bytes) junk (however many bytes it takes to complete a block) */ +@NotThreadSafe public class BlockedInputStream extends InputStream { byte[] array; // ReadPos is where to start reading diff --git a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/DistributedReadWriteLock.java b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/DistributedReadWriteLock.java index da27d408a08..484426d8094 100644 --- a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/DistributedReadWriteLock.java +++ b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/DistributedReadWriteLock.java @@ -30,6 +30,8 @@ import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.Lock; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.util.UtilWaitThread; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -107,6 +109,7 @@ public interface QueueLock { private static final Logger log = LoggerFactory.getLogger(DistributedReadWriteLock.class); + @NotThreadSafe static class ReadLock implements Lock { QueueLock qlock; diff --git a/core/src/main/java/org/apache/accumulo/core/file/BloomFilterLayer.java b/core/src/main/java/org/apache/accumulo/core/file/BloomFilterLayer.java index 7650ef8572d..c9eb55420aa 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/BloomFilterLayer.java +++ b/core/src/main/java/org/apache/accumulo/core/file/BloomFilterLayer.java @@ -35,6 +35,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.atomic.AtomicBoolean; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.bloomfilter.DynamicBloomFilter; import org.apache.accumulo.core.classloader.ClassLoaderUtil; import org.apache.accumulo.core.conf.AccumuloConfiguration; @@ -346,6 +348,7 @@ public void close() { } } + @NotThreadSafe public static class Reader implements FileSKVIterator { private final BloomFilterLoader bfl; diff --git a/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/SeekableByteArrayInputStream.java b/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/SeekableByteArrayInputStream.java index c231e88b942..732123d1990 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/SeekableByteArrayInputStream.java +++ b/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/SeekableByteArrayInputStream.java @@ -23,12 +23,15 @@ import java.io.IOException; import java.io.InputStream; +import javax.annotation.concurrent.NotThreadSafe; + import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; /** * This class is like byte array input stream with two differences. It supports seeking and avoids * synchronization. */ +@NotThreadSafe public class SeekableByteArrayInputStream extends InputStream { // making this volatile for the following case diff --git a/core/src/main/java/org/apache/accumulo/core/file/map/MapFileOperations.java b/core/src/main/java/org/apache/accumulo/core/file/map/MapFileOperations.java index c2da8c00665..26d24e5846a 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/map/MapFileOperations.java +++ b/core/src/main/java/org/apache/accumulo/core/file/map/MapFileOperations.java @@ -24,6 +24,8 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Range; @@ -43,6 +45,7 @@ public class MapFileOperations extends FileOperations { private static final String MSG = "Map files are not supported"; + @NotThreadSafe public static class RangeIterator implements FileSKVIterator { SortedKeyValueIterator reader; diff --git a/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java b/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java index 9e2d4633c26..55f64c9c25d 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java +++ b/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java @@ -40,6 +40,8 @@ import java.util.TreeMap; import java.util.concurrent.atomic.AtomicBoolean; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.SampleNotPresentException; import org.apache.accumulo.core.client.sample.Sampler; import org.apache.accumulo.core.client.sample.SamplerConfiguration; @@ -557,6 +559,7 @@ public void close() throws IOException { } } + @NotThreadSafe public static class Writer implements FileSKVWriter { public static final int MAX_CF_IN_DLG = 1000; @@ -754,6 +757,7 @@ public long getLength() { } } + @NotThreadSafe private static class LocalityGroupReader extends LocalityGroup implements FileSKVIterator { private final CachableBlockFile.Reader reader; diff --git a/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/SimpleBufferedOutputStream.java b/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/SimpleBufferedOutputStream.java index 87e96a602d8..e1bd64ed78f 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/SimpleBufferedOutputStream.java +++ b/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/SimpleBufferedOutputStream.java @@ -22,10 +22,13 @@ import java.io.IOException; import java.io.OutputStream; +import javax.annotation.concurrent.NotThreadSafe; + /** * A simplified BufferedOutputStream with borrowed buffer, and allow users to see how much data have * been buffered. */ +@NotThreadSafe class SimpleBufferedOutputStream extends FilterOutputStream { protected byte[] buf; // the borrowed buffer protected int count = 0; // bytes used in buffer. diff --git a/core/src/main/java/org/apache/accumulo/core/file/streams/BoundedRangeFileInputStream.java b/core/src/main/java/org/apache/accumulo/core/file/streams/BoundedRangeFileInputStream.java index d9f41862ae0..ff6a7be71ba 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/streams/BoundedRangeFileInputStream.java +++ b/core/src/main/java/org/apache/accumulo/core/file/streams/BoundedRangeFileInputStream.java @@ -21,6 +21,8 @@ import java.io.IOException; import java.io.InputStream; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.hadoop.fs.Seekable; /** @@ -28,6 +30,7 @@ * regular input stream. One can create multiple BoundedRangeFileInputStream on top of the same * FSDataInputStream and they would not interfere with each other. */ +@NotThreadSafe public class BoundedRangeFileInputStream extends InputStream { private volatile boolean closed = false; diff --git a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/ColumnFamilySkippingIterator.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/ColumnFamilySkippingIterator.java index 3745bf0fc33..5c48a581e0c 100644 --- a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/ColumnFamilySkippingIterator.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/ColumnFamilySkippingIterator.java @@ -25,6 +25,8 @@ import java.util.TreeSet; import java.util.concurrent.atomic.AtomicBoolean; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.Column; import org.apache.accumulo.core.data.Key; @@ -36,6 +38,7 @@ import org.apache.accumulo.core.iterators.ServerSkippingIterator; import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +@NotThreadSafe public class ColumnFamilySkippingIterator extends ServerSkippingIterator implements InterruptibleIterator { diff --git a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/StatsIterator.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/StatsIterator.java index a2f8da5975f..4800ffabf6c 100644 --- a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/StatsIterator.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/StatsIterator.java @@ -26,6 +26,8 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.LongAdder; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Range; @@ -34,6 +36,7 @@ import org.apache.accumulo.core.iterators.ServerWrappingIterator; import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +@NotThreadSafe public class StatsIterator extends ServerWrappingIterator { private int numRead = 0; diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java index 41189e8d663..6c2bcbfe4c8 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java @@ -44,6 +44,8 @@ import java.util.stream.Stream; import java.util.stream.StreamSupport; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.AccumuloClient; import org.apache.accumulo.core.client.BatchScanner; import org.apache.accumulo.core.client.IsolatedScanner; @@ -89,6 +91,7 @@ */ public class TabletsMetadata implements Iterable, AutoCloseable { + @NotThreadSafe public static class Builder implements TableRangeOptions, TableOptions, RangeOptions, Options { private final List families = new ArrayList<>(); diff --git a/core/src/main/java/org/apache/accumulo/core/singletons/SingletonManager.java b/core/src/main/java/org/apache/accumulo/core/singletons/SingletonManager.java index 5f3e151b58f..a8dee7fffb0 100644 --- a/core/src/main/java/org/apache/accumulo/core/singletons/SingletonManager.java +++ b/core/src/main/java/org/apache/accumulo/core/singletons/SingletonManager.java @@ -27,6 +27,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** * This class automates management of static singletons that maintain state for Accumulo clients. * Historically, Accumulo client code that used Connector had no control over these singletons. The @@ -81,6 +83,8 @@ public enum Mode { private static List services; @VisibleForTesting + @SuppressFBWarnings(value = "AT_NONATOMIC_64BIT_PRIMITIVE", + justification = "only called in static init block and testing, no sync needed") static void reset() { reservations = 0; mode = Mode.CLIENT; diff --git a/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java b/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java index 99bfd8472c2..8750e59aa61 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java @@ -169,8 +169,10 @@ public String toString() { private int maxFilesToCompact; private double lowestRatio; - @SuppressFBWarnings(value = {"UWF_UNWRITTEN_FIELD", "NP_UNWRITTEN_FIELD"}, - justification = "Field is written by Gson") + @SuppressFBWarnings( + value = {"UWF_UNWRITTEN_FIELD", "NP_UNWRITTEN_FIELD", "AT_NONATOMIC_64BIT_PRIMITIVE"}, + justification = "UWF_UNWRITTEN_FIELD and NP_UNWRITTEN_FIELD: Field is written by Gson. " + + "AT_NONATOMIC_64BIT_PRIMITIVE: Fields modified here are initialized once, and read-only after.") @Override public void init(InitParameters params) { @@ -243,6 +245,8 @@ public void init(InitParameters params) { } @SuppressWarnings("removal") + @SuppressFBWarnings(value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE", + justification = "only called in init") private void determineMaxFilesToCompact(InitParameters params) { String fqo = params.getFullyQualifiedOption("maxOpen"); if (!params.getServiceEnvironment().getConfiguration().isSet(fqo) diff --git a/core/src/main/java/org/apache/accumulo/core/spi/crypto/AESCryptoService.java b/core/src/main/java/org/apache/accumulo/core/spi/crypto/AESCryptoService.java index 555103fd983..86aa95b26d2 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/crypto/AESCryptoService.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/crypto/AESCryptoService.java @@ -117,6 +117,8 @@ protected Cipher initialValue() { }; @Override + @SuppressFBWarnings(value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE", + justification = "Fields modified here are initialized once, and read-only after.") public void init(Map conf) throws CryptoException { ensureNotInit(); String keyLocation = Objects.requireNonNull(conf.get(KEY_URI_PROPERTY), diff --git a/core/src/main/java/org/apache/accumulo/core/util/CountingInputStream.java b/core/src/main/java/org/apache/accumulo/core/util/CountingInputStream.java index 15d41b18d70..5f6a03871ed 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/CountingInputStream.java +++ b/core/src/main/java/org/apache/accumulo/core/util/CountingInputStream.java @@ -18,6 +18,8 @@ import java.io.InputStream; import java.util.Objects; +import javax.annotation.concurrent.NotThreadSafe; + /** * This class was copied from Guava and modified. If this class was not final in Guava it could have * been extended. Guava has issue 590 open about this. @@ -26,6 +28,7 @@ * * @author Chris Nokleberg */ +@NotThreadSafe public final class CountingInputStream extends FilterInputStream { private long count; diff --git a/pom.xml b/pom.xml index 6e07454736c..7fff4f11f6c 100644 --- a/pom.xml +++ b/pom.xml @@ -781,7 +781,7 @@ under the License. true 1024 16 - ConstructorThrow,SharedVariableAtomicityDetector + ConstructorThrow -Dcom.overstock.findbugs.ignore=com.google.common.util.concurrent.RateLimiter,com.google.common.hash.Hasher,com.google.common.hash.HashCode,com.google.common.hash.HashFunction,com.google.common.hash.Hashing,com.google.common.cache.Cache,com.google.common.io.CountingOutputStream,com.google.common.io.ByteStreams,com.google.common.cache.LoadingCache,com.google.common.base.Stopwatch,com.google.common.cache.RemovalNotification,com.google.common.util.concurrent.Uninterruptibles,com.google.common.reflect.ClassPath,com.google.common.reflect.ClassPath$ClassInfo,com.google.common.base.Throwables,com.google.common.collect.Iterators diff --git a/server/base/pom.xml b/server/base/pom.xml index bef6dd39600..ae1a1d5a14e 100644 --- a/server/base/pom.xml +++ b/server/base/pom.xml @@ -44,6 +44,10 @@ auto-service true + + com.google.code.findbugs + jsr305 + com.google.code.gson gson diff --git a/server/base/src/main/java/org/apache/accumulo/server/compaction/CountingIterator.java b/server/base/src/main/java/org/apache/accumulo/server/compaction/CountingIterator.java index 4c768ba6f00..f029f022848 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/compaction/CountingIterator.java +++ b/server/base/src/main/java/org/apache/accumulo/server/compaction/CountingIterator.java @@ -23,12 +23,15 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicLong; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.iterators.IteratorEnvironment; import org.apache.accumulo.core.iterators.SortedKeyValueIterator; import org.apache.accumulo.core.iterators.WrappingIterator; +@NotThreadSafe public class CountingIterator extends WrappingIterator { private long count; diff --git a/server/base/src/main/java/org/apache/accumulo/server/fs/FileManager.java b/server/base/src/main/java/org/apache/accumulo/server/fs/FileManager.java index b2f9daeccd3..7a69bb8537b 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/fs/FileManager.java +++ b/server/base/src/main/java/org/apache/accumulo/server/fs/FileManager.java @@ -33,6 +33,8 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.SampleNotPresentException; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.Key; @@ -386,6 +388,7 @@ private void releaseReaders(KeyExtent tablet, List readers, } + @NotThreadSafe static class FileDataSource implements DataSource { private SortedKeyValueIterator iter; diff --git a/server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReportingIterator.java b/server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReportingIterator.java index 4344c6ba975..e72cd6dab45 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReportingIterator.java +++ b/server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReportingIterator.java @@ -23,6 +23,8 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Range; @@ -33,6 +35,7 @@ import org.apache.accumulo.core.iteratorsImpl.system.InterruptibleIterator; import org.apache.accumulo.server.ServerContext; +@NotThreadSafe public class ProblemReportingIterator implements InterruptibleIterator { private final SortedKeyValueIterator source; private boolean sawError = false; diff --git a/server/base/src/main/java/org/apache/accumulo/server/security/delegation/AuthenticationTokenKeyManager.java b/server/base/src/main/java/org/apache/accumulo/server/security/delegation/AuthenticationTokenKeyManager.java index 5348e3bb9bd..7596e4e172d 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/security/delegation/AuthenticationTokenKeyManager.java +++ b/server/base/src/main/java/org/apache/accumulo/server/security/delegation/AuthenticationTokenKeyManager.java @@ -26,6 +26,8 @@ import com.google.common.annotations.VisibleForTesting; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** * Service that handles generation of the secret key used to create delegation tokens. */ @@ -93,6 +95,9 @@ public void run() { } @VisibleForTesting + @SuppressFBWarnings( + value = {"AT_STALE_THREAD_WRITE_OF_PRIMITIVE", "AT_NONATOMIC_64BIT_PRIMITIVE"}, + justification = "only called from run() and testing") void updateStateFromCurrentKeys() { try { List currentKeys = keyDistributor.getCurrentKeys(); @@ -138,6 +143,9 @@ int getIdSeq() { * * @param now The current time in millis since epoch. */ + @SuppressFBWarnings( + value = {"AT_STALE_THREAD_WRITE_OF_PRIMITIVE", "AT_NONATOMIC_64BIT_PRIMITIVE"}, + justification = "only called from run() and testing") void _run(long now) { // clear any expired keys int removedKeys = secretManager.removeExpiredKeys(keyDistributor); diff --git a/server/base/src/main/java/org/apache/accumulo/server/tablets/TabletTime.java b/server/base/src/main/java/org/apache/accumulo/server/tablets/TabletTime.java index 45d3870db15..b83fe6bdbe5 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/tablets/TabletTime.java +++ b/server/base/src/main/java/org/apache/accumulo/server/tablets/TabletTime.java @@ -27,6 +27,8 @@ import org.apache.accumulo.server.data.ServerMutation; import org.apache.accumulo.server.util.time.RelativeTime; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + public abstract class TabletTime { public abstract void useMaxTimeFromWALog(long time); @@ -87,6 +89,8 @@ public MetadataTime getMetadataTime(long time) { } @Override + @SuppressFBWarnings(value = "AT_NONATOMIC_64BIT_PRIMITIVE", + justification = "this is only called in tablet constructor, so does not need to be done atomically") public void useMaxTimeFromWALog(long time) { if (time > lastTime) { lastTime = time; @@ -157,11 +161,7 @@ private LogicalTime(Long time) { @Override public void useMaxTimeFromWALog(long time) { - time++; - - if (this.nextTime.get() < time) { - this.nextTime.set(time); - } + this.nextTime.getAndUpdate(val -> Math.max(val, time + 1)); } @Override diff --git a/server/gc/pom.xml b/server/gc/pom.xml index 148e63b58e7..86fea18db88 100644 --- a/server/gc/pom.xml +++ b/server/gc/pom.xml @@ -36,6 +36,10 @@ auto-service true + + com.google.code.findbugs + jsr305 + com.google.guava guava diff --git a/server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java b/server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java index 22275c9e119..98a7e53ead4 100644 --- a/server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java +++ b/server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java @@ -42,6 +42,8 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Stream; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.client.AccumuloClient; import org.apache.accumulo.core.client.IsolatedScanner; @@ -91,6 +93,7 @@ /** * A single garbage collection performed on a table (Root, MD) or all User tables. */ +@NotThreadSafe public class GCRun implements GarbageCollectionEnvironment { // loggers are not static to support unique naming by level private final Logger log; diff --git a/server/manager/pom.xml b/server/manager/pom.xml index 5f1516cb2e4..89543ae7398 100644 --- a/server/manager/pom.xml +++ b/server/manager/pom.xml @@ -36,6 +36,10 @@ auto-service true + + com.google.code.findbugs + jsr305 + com.google.code.gson gson diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/LoadFiles.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/LoadFiles.java index e26db3c4e63..ee523485694 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/LoadFiles.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/LoadFiles.java @@ -38,6 +38,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.BatchWriter; import org.apache.accumulo.core.client.MutationsRejectedException; import org.apache.accumulo.core.clientImpl.bulk.Bulk; @@ -159,6 +161,7 @@ void start(Path bulkDir, Manager manager, long tid, boolean setTime) throws Exce abstract long finish() throws Exception; } + @NotThreadSafe static class OnlineLoader extends Loader { private final int maxConnections; diff --git a/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java b/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java index f9672ed1965..5f2b0af0d35 100644 --- a/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java +++ b/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java @@ -130,14 +130,14 @@ public static void main(String[] args) throws Exception { } private final AtomicLong lastRecalc = new AtomicLong(0L); - private double totalIngestRate = 0.0; - private double totalQueryRate = 0.0; - private double totalScanRate = 0.0; - private long totalEntries = 0L; - private int totalTabletCount = 0; - private long totalHoldTime = 0; - private long totalLookups = 0; - private int totalTables = 0; + private volatile double totalIngestRate = 0.0; + private volatile double totalQueryRate = 0.0; + private volatile double totalScanRate = 0.0; + private volatile long totalEntries = 0L; + private volatile int totalTabletCount = 0; + private volatile long totalHoldTime = 0; + private volatile long totalLookups = 0; + private volatile int totalTables = 0; private final AtomicBoolean monitorInitialized = new AtomicBoolean(false); private static List> newMaxList() { @@ -998,10 +998,6 @@ public Optional getCoordinatorHost() { return coordinatorHost; } - public int getLivePort() { - return livePort; - } - @Override public ServiceLock getLock() { return monitorLock; diff --git a/server/tserver/pom.xml b/server/tserver/pom.xml index 40622e37a18..ec3b5de64e4 100644 --- a/server/tserver/pom.xml +++ b/server/tserver/pom.xml @@ -44,6 +44,10 @@ auto-service true + + com.google.code.findbugs + jsr305 + com.google.guava guava diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java index 306a4e7d270..c8182e6e1f1 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java @@ -37,6 +37,8 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.SampleNotPresentException; import org.apache.accumulo.core.client.sample.Sampler; import org.apache.accumulo.core.conf.AccumuloConfiguration; @@ -531,6 +533,7 @@ public synchronized long getNumEntries() { private final Set activeIters = Collections.synchronizedSet(new HashSet<>()); + @NotThreadSafe class MemoryDataSource implements DataSource { private boolean switched = false; diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java index 0b8a6816c29..0173b2b95be 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java @@ -34,6 +34,8 @@ import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.SampleNotPresentException; import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.ColumnUpdate; @@ -534,6 +536,7 @@ public void delete() { } } + @NotThreadSafe private static class NMSKVIter implements InterruptibleIterator { private ConcurrentIterator iter; diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java index 748d74705bd..38f9c80d87b 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java @@ -412,7 +412,7 @@ public int getOpenFiles() { public static class AssignmentWatcher implements Runnable { private static final Logger log = LoggerFactory.getLogger(AssignmentWatcher.class); - private static long longAssignments = 0; + private static volatile long longAssignments = 0; private final Map activeAssignments; private final AccumuloConfiguration conf; diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java index 1e6c408ee5d..4d1287cf7d1 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java @@ -27,6 +27,8 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.dataImpl.thrift.IterInfo; @@ -62,6 +64,7 @@ import io.opentelemetry.api.trace.Span; +@NotThreadSafe class ScanDataSource implements DataSource { private static final Logger log = LoggerFactory.getLogger(ScanDataSource.class); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Scanner.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Scanner.java index 9e2a7c7af03..adc93997101 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Scanner.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Scanner.java @@ -41,6 +41,8 @@ import com.google.common.base.Preconditions; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + public class Scanner { private static final Logger log = LoggerFactory.getLogger(Scanner.class); @@ -213,6 +215,8 @@ public Thread getLockOwner() { * read is in progress), it interrupts the reading thread. This ensures the reading thread can * finish its current operation and release the lock, allowing close to finish. */ + @SuppressFBWarnings(value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE", + justification = "accesses non-volatile/non-atomic vars only when lock is held") public boolean close() { interruptFlag.set(true); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java index d7ff6a7c8dc..fd5e9bc9ef8 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java @@ -226,8 +226,8 @@ enum CompactionState { private final Rate ingestByteRate = new Rate(0.95); private final Rate scannedRate = new Rate(0.95); - private long lastMinorCompactionFinishTime = 0; - private long lastMapFileImportTime = 0; + private volatile long lastMinorCompactionFinishTime = 0; + private volatile long lastMapFileImportTime = 0; private volatile long numEntries = 0; private volatile long numEntriesInMemory = 0; diff --git a/server/tserver/src/test/java/org/apache/accumulo/tserver/tablet/CompactableImplFileManagerTest.java b/server/tserver/src/test/java/org/apache/accumulo/tserver/tablet/CompactableImplFileManagerTest.java index 743ac8feca3..aa9d8189076 100644 --- a/server/tserver/src/test/java/org/apache/accumulo/tserver/tablet/CompactableImplFileManagerTest.java +++ b/server/tserver/src/test/java/org/apache/accumulo/tserver/tablet/CompactableImplFileManagerTest.java @@ -37,6 +37,8 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.admin.compaction.CompactableFile; import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.dataImpl.KeyExtent; @@ -478,6 +480,7 @@ private void assertNoCandidates(TestFileManager fileMgr, Set t } + @NotThreadSafe static class TestFileManager extends CompactableImpl.FileManager { public static final Duration SELECTION_EXPIRATION = Duration.ofMinutes(2); diff --git a/test/pom.xml b/test/pom.xml index 4430f6d929e..4db0359b7a5 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -46,6 +46,10 @@ auto-service true + + com.google.code.findbugs + jsr305 + com.google.code.gson gson diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ErrorThrowingIterator.java b/test/src/main/java/org/apache/accumulo/test/functional/ErrorThrowingIterator.java index d9cb2c0e8dd..13b6194c0fc 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ErrorThrowingIterator.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ErrorThrowingIterator.java @@ -23,6 +23,8 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Range; @@ -37,6 +39,7 @@ * Iterator used in tests *and* the test class must spawn a new MAC instance for each test since the * timesThrown variable is static. */ +@NotThreadSafe public class ErrorThrowingIterator extends WrappingIterator { public static final String TIMES = "error.throwing.iterator.times"; diff --git a/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java b/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java index 62e034bbef5..9256d518d42 100644 --- a/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java +++ b/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java @@ -36,6 +36,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import javax.annotation.concurrent.NotThreadSafe; + import org.apache.accumulo.core.client.Accumulo; import org.apache.accumulo.core.client.AccumuloClient; import org.apache.accumulo.core.client.Scanner; @@ -248,6 +250,7 @@ public int runTest() throws Exception { threadPool.shutdown(); } + @NotThreadSafe private abstract static class Test implements Runnable { private int count;