Skip to content
This repository was archived by the owner on Oct 5, 2021. It is now read-only.

Commit 3db96c7

Browse files
Alexander PatrikalakisAlexander Patrikalakis
Alexander Patrikalakis
authored and
Alexander Patrikalakis
committed
Address all FindBugs warnings
1 parent f978108 commit 3db96c7

15 files changed

+96
-65
lines changed

‎pom.xml

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,31 @@
195195
<artifactId>lombok</artifactId>
196196
<version>${lombok.version}</version>
197197
</dependency>
198+
<dependency>
199+
<groupId>com.google.code.findbugs</groupId>
200+
<artifactId>findbugs</artifactId>
201+
<version>3.0.1</version>
202+
</dependency>
198203
</dependencies>
199204
<build>
200205
<plugins>
201-
<!-- TODO uncomment to find and fix more checkstyle issues -->
206+
<plugin>
207+
<groupId>org.codehaus.mojo</groupId>
208+
<artifactId>findbugs-maven-plugin</artifactId>
209+
<version>3.0.4</version>
210+
<configuration>
211+
<effort>Max</effort>
212+
<threshold>Low</threshold>
213+
<xmlOutput>true</xmlOutput>
214+
</configuration>
215+
<executions>
216+
<execution>
217+
<goals>
218+
<goal>check</goal>
219+
</goals>
220+
</execution>
221+
</executions>
222+
</plugin>
202223
<plugin>
203224
<groupId>org.apache.maven.plugins</groupId>
204225
<artifactId>maven-checkstyle-plugin</artifactId>

‎src/main/java/com/amazon/janusgraph/diskstorage/dynamodb/BackendRuntimeException.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
package com.amazon.janusgraph.diskstorage.dynamodb;
1616

1717

18-
import java.util.Optional;
19-
2018
import org.janusgraph.diskstorage.BackendException;
2119

2220
/**
@@ -34,7 +32,12 @@ public BackendRuntimeException(final BackendException e) {
3432
}
3533

3634
public BackendException getBackendException() {
37-
return Optional.ofNullable((BackendException) super.getCause()).orElse(null);
35+
final Throwable throwable = super.getCause();
36+
if (throwable instanceof BackendException) {
37+
return (BackendException) throwable;
38+
} else {
39+
return null;
40+
}
3841
}
3942

4043
private static final long serialVersionUID = 6184087040805925812L;

‎src/main/java/com/amazon/janusgraph/diskstorage/dynamodb/Client.java

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,12 @@
2323
import java.util.List;
2424
import java.util.Map;
2525
import java.util.Set;
26-
import java.util.concurrent.ArrayBlockingQueue;
27-
import java.util.concurrent.ThreadFactory;
28-
import java.util.concurrent.ThreadPoolExecutor;
29-
import java.util.concurrent.TimeUnit;
3026

3127
import lombok.AccessLevel;
3228
import lombok.Getter;
3329
import lombok.NonNull;
3430
import org.janusgraph.diskstorage.configuration.Configuration;
3531
import org.janusgraph.graphdb.configuration.GraphDatabaseConfiguration;
36-
import org.janusgraph.util.stats.MetricManager;
3732

3833
import com.amazonaws.ClientConfiguration;
3934
import com.amazonaws.auth.AWSCredentials;
@@ -43,7 +38,6 @@
4338
import com.google.common.base.Strings;
4439
import com.google.common.util.concurrent.RateLimiter;
4540
import com.google.common.util.concurrent.RateLimiterCreator;
46-
import com.google.common.util.concurrent.ThreadFactoryBuilder;
4741

4842
/**
4943
* Operations setting up the DynamoDB client.
@@ -56,8 +50,6 @@ public class Client {
5650
private static final String VALIDATE_CREDENTIALS_CLASS_NAME = "Must provide either an AWSCredentials or AWSCredentialsProvider fully qualified class name";
5751
private static final double DEFAULT_BURST_BUCKET_SIZE_IN_SECONDS = 300.0;
5852

59-
protected final MetricManager metrics = MetricManager.INSTANCE;
60-
6153
private final Map<String, Long> capacityRead = new HashMap<>();
6254
private final Map<String, Long> capacityWrite = new HashMap<>();
6355
private final Map<String, BackendDataModel> dataModelMap = new HashMap<>();
@@ -153,23 +145,6 @@ public Client(final Configuration config) {
153145
clientConfig, config, readRateLimit, writeRateLimit, maxRetries, retryMillis, prefix, metricsPrefix, controlPlaneRateLimiter);
154146
}
155147

156-
static ThreadPoolExecutor getPoolFromNs(final Configuration ns) {
157-
final int maxQueueSize = ns.get(Constants.DYNAMODB_CLIENT_EXECUTOR_QUEUE_MAX_LENGTH);
158-
final ThreadFactory factory = new ThreadFactoryBuilder().setNameFormat("getDelegate-%d").build();
159-
//begin adaptation of constructor at
160-
//https://github.com/buka/titan/blob/master/src/main/java/com/thinkaurelius/titan/diskstorage/dynamodb/DynamoDBClient.java#L104
161-
final int maxPoolSize = ns.get(Constants.DYNAMODB_CLIENT_EXECUTOR_MAX_POOL_SIZE);
162-
final int corePoolSize = ns.get(Constants.DYNAMODB_CLIENT_EXECUTOR_CORE_POOL_SIZE);
163-
final long keepAlive = ns.get(Constants.DYNAMODB_CLIENT_EXECUTOR_KEEP_ALIVE);
164-
final ThreadPoolExecutor executor = new ThreadPoolExecutor(corePoolSize, maxPoolSize, keepAlive,
165-
TimeUnit.MILLISECONDS, new ArrayBlockingQueue<>(maxQueueSize), factory, new ThreadPoolExecutor.CallerRunsPolicy());
166-
//end adaptation of constructor at
167-
//https://github.com/buka/titan/blob/master/src/main/java/com/thinkaurelius/titan/diskstorage/dynamodb/DynamoDBClient.java#L104
168-
executor.allowCoreThreadTimeOut(false);
169-
executor.prestartAllCoreThreads();
170-
return executor;
171-
}
172-
173148
private void setupStore(final Configuration config,
174149
final Map<String, RateLimiter> readRateLimit, final Map<String, RateLimiter> writeRateLimit, final String store) {
175150

‎src/main/java/com/amazon/janusgraph/diskstorage/dynamodb/Constants.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import static org.janusgraph.diskstorage.configuration.ConfigOption.Type.FIXED;
1919
import static org.janusgraph.diskstorage.configuration.ConfigOption.Type.LOCAL;
2020

21-
import java.util.Arrays;
2221
import java.util.List;
2322

2423
import org.janusgraph.diskstorage.Backend;
@@ -28,6 +27,7 @@
2827

2928
import com.amazonaws.ClientConfiguration;
3029
import com.google.common.base.Predicates;
30+
import com.google.common.collect.ImmutableList;
3131

3232
/**
3333
* Constants for the DynamoDB backend.
@@ -49,7 +49,7 @@ private Constants() {
4949
public static final String HEX_PREFIX = "0x";
5050
public static final String JANUSGRAPH_USER_AGENT = "dynamodb-janusgraph010-storage-backend_1.0.0";
5151

52-
public static final List<String> REQUIRED_BACKEND_STORES = Arrays.asList(Backend.EDGESTORE_NAME, //
52+
public static final List<String> REQUIRED_BACKEND_STORES = ImmutableList.of(Backend.EDGESTORE_NAME, //
5353
Backend.INDEXSTORE_NAME,
5454
Backend.SYSTEM_TX_LOG_NAME,
5555
Backend.SYSTEM_MGMT_LOG_NAME,

‎src/main/java/com/amazon/janusgraph/diskstorage/dynamodb/DynamoDbDelegate.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,14 @@
2727
import java.util.Map.Entry;
2828
import java.util.Optional;
2929
import java.util.Set;
30+
import java.util.concurrent.ArrayBlockingQueue;
3031
import java.util.concurrent.CompletionService;
3132
import java.util.concurrent.ExecutionException;
3233
import java.util.concurrent.ExecutorCompletionService;
3334
import java.util.concurrent.Future;
35+
import java.util.concurrent.ThreadFactory;
3436
import java.util.concurrent.ThreadPoolExecutor;
37+
import java.util.concurrent.TimeUnit;
3538

3639
import lombok.Getter;
3740
import lombok.extern.slf4j.Slf4j;
@@ -102,6 +105,7 @@
102105
import com.google.common.collect.Lists;
103106
import com.google.common.collect.Maps;
104107
import com.google.common.util.concurrent.RateLimiter;
108+
import com.google.common.util.concurrent.ThreadFactoryBuilder;
105109

106110
/**
107111
* A wrapper on top of the DynamoDB client API that self-throttles using metric-based and context-aware
@@ -142,7 +146,7 @@ public class DynamoDbDelegate {
142146
public static final int BATCH_WRITE_MAX_NUMBER_OF_ITEMS = 25;
143147

144148
private final AmazonDynamoDB client;
145-
private static ThreadPoolExecutor clientThreadPool = null;
149+
private final ThreadPoolExecutor clientThreadPool;
146150
private final Map<String, RateLimiter> readRateLimit;
147151
private final Map<String, RateLimiter> writeRateLimit;
148152
private final RateLimiter controlPlaneRateLimiter;
@@ -171,9 +175,7 @@ public DynamoDbDelegate(final String endpoint, final String region, final AWSCre
171175
}
172176
this.metricsPrefix = metricsPrefix;
173177
executorGaugeName = String.format("%s.%s_executor-queue-size", this.metricsPrefix, prefix);
174-
if (clientThreadPool == null) {
175-
clientThreadPool = Client.getPoolFromNs(titanConfig);
176-
}
178+
clientThreadPool = getPoolFromNs(titanConfig);
177179
if (!MetricManager.INSTANCE.getRegistry().getNames().contains(executorGaugeName)) {
178180
MetricManager.INSTANCE.getRegistry().register(executorGaugeName, (Gauge<Integer>) () -> clientThreadPool.getQueue().size());
179181
}
@@ -195,6 +197,23 @@ public DynamoDbDelegate(final String endpoint, final String region, final AWSCre
195197
this.listTablesApiName = String.format("%s_ListTables", prefix);
196198
}
197199

200+
static ThreadPoolExecutor getPoolFromNs(final Configuration ns) {
201+
final int maxQueueSize = ns.get(Constants.DYNAMODB_CLIENT_EXECUTOR_QUEUE_MAX_LENGTH);
202+
final ThreadFactory factory = new ThreadFactoryBuilder().setNameFormat("getDelegate-%d").build();
203+
//begin adaptation of constructor at
204+
//https://github.com/buka/titan/blob/master/src/main/java/com/thinkaurelius/titan/diskstorage/dynamodb/DynamoDBClient.java#L104
205+
final int maxPoolSize = ns.get(Constants.DYNAMODB_CLIENT_EXECUTOR_MAX_POOL_SIZE);
206+
final int corePoolSize = ns.get(Constants.DYNAMODB_CLIENT_EXECUTOR_CORE_POOL_SIZE);
207+
final long keepAlive = ns.get(Constants.DYNAMODB_CLIENT_EXECUTOR_KEEP_ALIVE);
208+
final ThreadPoolExecutor executor = new ThreadPoolExecutor(corePoolSize, maxPoolSize, keepAlive,
209+
TimeUnit.MILLISECONDS, new ArrayBlockingQueue<>(maxQueueSize), factory, new ThreadPoolExecutor.CallerRunsPolicy());
210+
//end adaptation of constructor at
211+
//https://github.com/buka/titan/blob/master/src/main/java/com/thinkaurelius/titan/diskstorage/dynamodb/DynamoDBClient.java#L104
212+
executor.allowCoreThreadTimeOut(false);
213+
executor.prestartAllCoreThreads();
214+
return executor;
215+
}
216+
198217
@VisibleForTesting
199218
static AwsClientBuilder.EndpointConfiguration getEndpointConfiguration(final Optional<String> endpoint, final String signingRegion) {
200219
Preconditions.checkArgument(endpoint != null, "must provide an optional endpoint and not null");

‎src/main/java/com/amazon/janusgraph/diskstorage/dynamodb/DynamoDbStore.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,8 @@ private Collection<MutateWorker> createWorkersForAdditions(final StaticBuffer ha
268268
.rangeKey(rangeKey)
269269
.build();
270270

271-
final Expression updateExpression = new MultiUpdateExpressionBuilder().hashKey(hashKey)
271+
final Expression updateExpression = new MultiUpdateExpressionBuilder(txh).hashKey(hashKey)
272272
.rangeKey(rangeKey)
273-
.transaction(txh)
274273
.value(addition.getValue())
275274
.build();
276275

@@ -291,9 +290,8 @@ private Collection<MutateWorker> createWorkersForDeletions(final StaticBuffer ha
291290
.rangeKey(rangeKey)
292291
.build();
293292

294-
final Expression updateExpression = new MultiUpdateExpressionBuilder().hashKey(hashKey)
293+
final Expression updateExpression = new MultiUpdateExpressionBuilder(txh).hashKey(hashKey)
295294
.rangeKey(rangeKey)
296-
.transaction(txh)
297295
.build();
298296

299297
final DeleteItemRequest request = super.createDeleteItemRequest().withKey(keys)

‎src/main/java/com/amazon/janusgraph/diskstorage/dynamodb/Expression.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ public class Expression {
3636
@Getter
3737
private final String conditionExpression;
3838
private final Map<String, AttributeValue> attributeValues;
39-
private final Map<String, String> attributeNames;
4039

4140
public Map<String, AttributeValue> getAttributeValues() {
4241
// DynamoDB expects null expression maps when they are empty.

‎src/main/java/com/amazon/janusgraph/diskstorage/dynamodb/builder/ConditionExpressionBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public Expression build() {
7171
sb.append(" AND (").append(conditionExpressions.get(Constants.JANUSGRAPH_RANGE_KEY)).append(")");
7272
}
7373
return new Expression(null /*updateExpression*/, sb.toString(),
74-
expressionAttributeValues, null /*expressionAttributeNames*/);
74+
expressionAttributeValues);
7575
}
7676

7777
private ConditionExpressionBuilder between(final String key, final StaticBuffer start, final StaticBuffer end) {

‎src/main/java/com/amazon/janusgraph/diskstorage/dynamodb/builder/EntryBuilder.java

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414
*/
1515
package com.amazon.janusgraph.diskstorage.dynamodb.builder;
1616

17-
import java.util.ArrayList;
1817
import java.util.Collections;
1918
import java.util.List;
2019
import java.util.Map;
20+
import java.util.stream.Collectors;
2121

2222
import org.janusgraph.diskstorage.Entry;
2323
import org.janusgraph.diskstorage.StaticBuffer;
@@ -56,7 +56,6 @@ public List<Entry> buildAll() {
5656
if (null == item) {
5757
return Collections.emptyList();
5858
}
59-
final List<Entry> filteredEntries = new ArrayList<>(item.size());
6059
final Entry sliceStartEntry;
6160
final Entry sliceEndEntry;
6261
if (slice) {
@@ -66,20 +65,19 @@ public List<Entry> buildAll() {
6665
sliceStartEntry = null;
6766
sliceEndEntry = null;
6867
}
69-
for (String column : item.keySet()) {
70-
final StaticBuffer columnKey = decodeKey(column);
71-
final AttributeValue valueValue = item.get(column);
72-
final StaticBuffer value = decodeValue(valueValue);
73-
final Entry entry = StaticArrayEntry.of(columnKey, value);
74-
if (!slice || entry.compareTo(sliceStartEntry) >= 0 && entry.compareTo(sliceEndEntry) < 0) {
75-
filteredEntries.add(StaticArrayEntry.of(columnKey, value));
76-
}
77-
}
78-
7968
//TODO(alexp) Arrays.parallelSort(filteredEntries) in JDK 8? Can you switch to java 8?
8069
//https://github.com/awslabs/dynamodb-titan-storage-backend/issues/159
81-
Collections.sort(filteredEntries);
82-
return filteredEntries.subList(0, Math.min(filteredEntries.size(), limit));
70+
return item.entrySet().stream()
71+
.map(entry -> {
72+
final StaticBuffer columnKey = decodeKey(entry.getKey());
73+
final AttributeValue valueValue = entry.getValue();
74+
final StaticBuffer value = decodeValue(valueValue);
75+
return StaticArrayEntry.of(columnKey, value);
76+
})
77+
.filter(entry -> !slice || entry.compareTo(sliceStartEntry) >= 0 && entry.compareTo(sliceEndEntry) < 0)
78+
.sorted()
79+
.limit(limit)
80+
.collect(Collectors.toList());
8381
}
8482

8583
public Entry build() {

‎src/main/java/com/amazon/janusgraph/diskstorage/dynamodb/builder/FilterExpressionBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public Expression build() {
6969
expressionAttributeValues.put(endVar, encodeKeyAsAttributeValue(endValue));
7070

7171
return new Expression(null /*updateExpression*/, sb.toString(),
72-
expressionAttributeValues, null /*expressionAttributeNames*/);
72+
expressionAttributeValues);
7373
}
7474

7575
}

0 commit comments

Comments
 (0)