You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by ct...@apache.org on 2020/09/16 00:09:56 UTC
[accumulo] branch main updated: Clean up a few forEach loops (#1705)
This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new bc1d651 Clean up a few forEach loops (#1705)
bc1d651 is described below
commit bc1d6515e141457248b3b6f99ddcf136a1c621de
Author: Christopher Tubbs <ct...@apache.org>
AuthorDate: Tue Sep 15 20:09:44 2020 -0400
Clean up a few forEach loops (#1705)
Miscellaneous minor clean up found while working on unrelated code,
including:
* Using forEach loops on collections to streamline loops
* Inline one-time-use simple private methods
* Remove braces in some simple one-statement lambdas
---
.../client/mapreduce/AccumuloOutputFormat.java | 6 ++--
.../core/clientImpl/TableOperationsImpl.java | 7 ++--
.../core/clientImpl/TabletServerBatchWriter.java | 11 ++----
.../apache/accumulo/core/conf/IterConfigUtil.java | 2 --
.../format/ShardedTableDistributionFormatter.java | 3 +-
.../core/clientImpl/TabletLocatorImplTest.java | 10 ++----
.../hadoopImpl/mapred/AccumuloRecordWriter.java | 7 ++--
.../hadoopImpl/mapreduce/AccumuloRecordWriter.java | 7 ++--
.../accumulo/server/client/BulkImporter.java | 41 ++++++++--------------
.../balancer/HostRegexTableLoadBalancer.java | 6 ++--
.../server/master/balancer/TableLoadBalancer.java | 7 ++--
.../accumulo/server/util/MetadataTableUtil.java | 19 ++++------
.../master/balancer/DefaultLoadBalancerTest.java | 8 ++---
.../org/apache/accumulo/tserver/FileManager.java | 12 +++----
14 files changed, 46 insertions(+), 100 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloOutputFormat.java b/core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloOutputFormat.java
index 43937b8..b9fdae4 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloOutputFormat.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloOutputFormat.java
@@ -551,10 +551,8 @@ public class AccumuloOutputFormat extends OutputFormat<Text,Mutation> {
} catch (MutationsRejectedException e) {
if (!e.getSecurityErrorCodes().isEmpty()) {
var tables = new HashMap<String,Set<SecurityErrorCode>>();
- e.getSecurityErrorCodes().forEach((tabletId, secSet) -> {
- var tableId = tabletId.getTableId().toString();
- tables.computeIfAbsent(tableId, p -> new HashSet<>()).addAll(secSet);
- });
+ e.getSecurityErrorCodes().forEach((table, code) -> tables
+ .computeIfAbsent(table.getTableId().toString(), k -> new HashSet<>()).addAll(code));
log.error("Not authorized to write to tables : " + tables);
}
diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
index 207e245..6c02fee 100644
--- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
+++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
@@ -1791,11 +1791,8 @@ public class TableOperationsImpl extends TableOperationsHelper {
if (groupedByRanges == null) {
Map<Range,List<TabletId>> tmp = new HashMap<>();
- groupedByTablets.forEach((table, rangeList) -> {
- for (Range range : rangeList) {
- tmp.computeIfAbsent(range, k -> new ArrayList<>()).add(table);
- }
- });
+ groupedByTablets.forEach((tabletId, rangeList) -> rangeList
+ .forEach(range -> tmp.computeIfAbsent(range, k -> new ArrayList<>()).add(tabletId)));
Map<Range,List<TabletId>> tmp2 = new HashMap<>();
for (Entry<Range,List<TabletId>> entry : tmp.entrySet()) {
diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java
index f27df42..4967e3f 100644
--- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java
+++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java
@@ -509,19 +509,14 @@ public class TabletServerBatchWriter implements AutoCloseable {
synchronized (this) {
somethingFailed = true;
- mergeAuthorizationFailures(this.authorizationFailures, authorizationFailures);
+ // add these authorizationFailures to those collected by this batch writer
+ authorizationFailures.forEach((ke, code) -> this.authorizationFailures
+ .computeIfAbsent(ke, k -> new HashSet<>()).add(code));
this.notifyAll();
}
}
}
- private void mergeAuthorizationFailures(Map<KeyExtent,Set<SecurityErrorCode>> source,
- Map<KeyExtent,SecurityErrorCode> addition) {
- addition.forEach((ke, sec) -> {
- source.computeIfAbsent(ke, p -> new HashSet<>()).add(sec);
- });
- }
-
private synchronized void updateServerErrors(String server, Exception e) {
somethingFailed = true;
this.serverSideErrors.add(server);
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/IterConfigUtil.java b/core/src/main/java/org/apache/accumulo/core/conf/IterConfigUtil.java
index d5f5ed6..bd5a4f5 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/IterConfigUtil.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/IterConfigUtil.java
@@ -115,9 +115,7 @@ public class IterConfigUtil {
} else if (suffixSplit.length == 3 && suffixSplit[1].equals("opt")) {
String iterName = suffixSplit[0];
String optName = suffixSplit[2];
-
allOptions.computeIfAbsent(iterName, k -> new HashMap<>()).put(optName, entry.getValue());
-
} else {
throw new IllegalArgumentException("Invalid iterator format: " + entry.getKey());
}
diff --git a/core/src/main/java/org/apache/accumulo/core/util/format/ShardedTableDistributionFormatter.java b/core/src/main/java/org/apache/accumulo/core/util/format/ShardedTableDistributionFormatter.java
index ee08a02..658bb44 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/format/ShardedTableDistributionFormatter.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/format/ShardedTableDistributionFormatter.java
@@ -54,8 +54,7 @@ public class ShardedTableDistributionFormatter extends AggregatingFormatter {
day = row.substring(semicolon, semicolon + 8);
}
String server = entry.getValue().toString();
- countsByDay.computeIfAbsent(day, k -> new HashSet<>());
- countsByDay.get(day).add(server);
+ countsByDay.computeIfAbsent(day, k -> new HashSet<>()).add(server);
}
}
diff --git a/core/src/test/java/org/apache/accumulo/core/clientImpl/TabletLocatorImplTest.java b/core/src/test/java/org/apache/accumulo/core/clientImpl/TabletLocatorImplTest.java
index 74e9786..228ea71 100644
--- a/core/src/test/java/org/apache/accumulo/core/clientImpl/TabletLocatorImplTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/clientImpl/TabletLocatorImplTest.java
@@ -294,7 +294,6 @@ public class TabletLocatorImplTest {
String row = (String) ol[0];
String server = (String) ol[1];
KeyExtent ke = (KeyExtent) ol[2];
-
emb.computeIfAbsent(server, k -> new HashMap<>()).computeIfAbsent(ke, k -> new ArrayList<>())
.add(row);
}
@@ -517,12 +516,8 @@ public class TabletLocatorImplTest {
static void createEmptyTablet(TServers tservers, String server, KeyExtent tablet) {
Map<KeyExtent,SortedMap<Key,Value>> tablets =
tservers.tservers.computeIfAbsent(server, k -> new HashMap<>());
-
- SortedMap<Key,Value> tabletData = tablets.get(tablet);
- if (tabletData == null) {
- tabletData = new TreeMap<>();
- tablets.put(tablet, tabletData);
- } else if (!tabletData.isEmpty()) {
+ SortedMap<Key,Value> tabletData = tablets.computeIfAbsent(tablet, k -> new TreeMap<>());
+ if (!tabletData.isEmpty()) {
throw new RuntimeException("Asked for empty tablet, but non empty tablet exists");
}
}
@@ -549,7 +544,6 @@ public class TabletLocatorImplTest {
String location, String instance) {
Map<KeyExtent,SortedMap<Key,Value>> tablets =
tservers.tservers.computeIfAbsent(server, k -> new HashMap<>());
-
SortedMap<Key,Value> tabletData = tablets.computeIfAbsent(tablet, k -> new TreeMap<>());
Text mr = ke.toMetaRow();
diff --git a/hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoopImpl/mapred/AccumuloRecordWriter.java b/hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoopImpl/mapred/AccumuloRecordWriter.java
index 6461c5c..4a2567a 100644
--- a/hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoopImpl/mapred/AccumuloRecordWriter.java
+++ b/hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoopImpl/mapred/AccumuloRecordWriter.java
@@ -186,11 +186,8 @@ public class AccumuloRecordWriter implements RecordWriter<Text,Mutation> {
} catch (MutationsRejectedException e) {
if (!e.getSecurityErrorCodes().isEmpty()) {
var tables = new HashMap<String,Set<SecurityErrorCode>>();
- e.getSecurityErrorCodes().forEach((tabletId, secSet) -> {
- String tableId = tabletId.getTableId().toString();
- tables.computeIfAbsent(tableId, p -> new HashSet<>()).addAll(secSet);
- });
-
+ e.getSecurityErrorCodes().forEach((tabletId, codes) -> tables
+ .computeIfAbsent(tabletId.getTableId().toString(), k -> new HashSet<>()).addAll(codes));
log.error("Not authorized to write to tables : " + tables);
}
diff --git a/hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoopImpl/mapreduce/AccumuloRecordWriter.java b/hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoopImpl/mapreduce/AccumuloRecordWriter.java
index 6a513a3..fbde6e4 100644
--- a/hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoopImpl/mapreduce/AccumuloRecordWriter.java
+++ b/hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoopImpl/mapreduce/AccumuloRecordWriter.java
@@ -187,11 +187,8 @@ public class AccumuloRecordWriter extends RecordWriter<Text,Mutation> {
} catch (MutationsRejectedException e) {
if (!e.getSecurityErrorCodes().isEmpty()) {
HashMap<String,Set<SecurityErrorCode>> tables = new HashMap<>();
- for (var ke : e.getSecurityErrorCodes().entrySet()) {
- String tableId = ke.getKey().getTableId().toString();
- tables.computeIfAbsent(tableId, k -> new HashSet<>()).addAll(ke.getValue());
- }
-
+ e.getSecurityErrorCodes().forEach((tabletId, codes) -> tables
+ .computeIfAbsent(tabletId.getTableId().toString(), k -> new HashSet<>()).addAll(codes));
log.error("Not authorized to write to tables : " + tables);
}
diff --git a/server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java b/server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java
index e3a2bb5..5156193 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java
@@ -464,11 +464,9 @@ public class BulkImporter {
failures.forEach(ke -> {
List<PathSize> mapFiles = assignmentsPerTablet.get(ke);
synchronized (assignmentFailures) {
- mapFiles.forEach(pathSize -> {
- assignmentFailures.computeIfAbsent(pathSize.path, k -> new ArrayList<>()).add(ke);
- });
+ mapFiles.forEach(pathSize -> assignmentFailures
+ .computeIfAbsent(pathSize.path, k -> new ArrayList<>()).add(ke));
}
-
log.info("Could not assign {} map files to tablet {} because : {}. Will retry ...",
mapFiles.size(), ke, message);
});
@@ -514,13 +512,10 @@ public class BulkImporter {
// group assignments by tablet
Map<KeyExtent,List<PathSize>> assignmentsPerTablet = new TreeMap<>();
-
- assignments.forEach((mapFile, tabletsToAssignMapFileTo) -> {
- tabletsToAssignMapFileTo.forEach(ai -> {
- assignmentsPerTablet.computeIfAbsent(ai.ke, k -> new ArrayList<>())
- .add(new PathSize(mapFile, ai.estSize));
- });
- });
+ assignments.forEach((mapFile, tabletsToAssignMapFileTo) -> tabletsToAssignMapFileTo
+ .forEach(assignmentInfo -> assignmentsPerTablet
+ .computeIfAbsent(assignmentInfo.ke, k -> new ArrayList<>())
+ .add(new PathSize(mapFile, assignmentInfo.estSize))));
// group assignments by tabletserver
@@ -528,27 +523,21 @@ public class BulkImporter {
TreeMap<String,Map<KeyExtent,List<PathSize>>> assignmentsPerTabletServer = new TreeMap<>();
- for (Entry<KeyExtent,List<PathSize>> entry : assignmentsPerTablet.entrySet()) {
- KeyExtent ke = entry.getKey();
+ assignmentsPerTablet.forEach((ke, pathSizes) -> {
String location = locations.get(ke);
-
if (location == null) {
- for (PathSize pathSize : entry.getValue()) {
- synchronized (assignmentFailures) {
- assignmentFailures.computeIfAbsent(pathSize.path, k -> new ArrayList<>()).add(ke);
- }
+ synchronized (assignmentFailures) {
+ pathSizes.forEach(pathSize -> assignmentFailures
+ .computeIfAbsent(pathSize.path, k -> new ArrayList<>()).add(ke));
}
-
log.warn(
"Could not assign {} map files to tablet {} because it had no location, will retry ...",
- entry.getValue().size(), ke);
-
- continue;
+ pathSizes.size(), ke);
+ } else {
+ assignmentsPerTabletServer.computeIfAbsent(location, k -> new TreeMap<>()).put(ke,
+ pathSizes);
}
-
- assignmentsPerTabletServer.computeIfAbsent(location, k -> new TreeMap<>()).put(entry.getKey(),
- entry.getValue());
- }
+ });
ExecutorService threadPool =
Executors.newFixedThreadPool(numThreads, new NamingThreadFactory("submit"));
diff --git a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java
index c265b38..40d8127 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java
@@ -350,10 +350,8 @@ public class HostRegexTableLoadBalancer extends TableLoadBalancer {
Map<String,SortedMap<TServerInstance,TabletServerStatus>> pools = splitCurrentByRegex(current);
// group the unassigned into tables
Map<TableId,Map<KeyExtent,TServerInstance>> groupedUnassigned = new HashMap<>();
- unassigned.forEach((keyExtent, tServerInstance) -> {
- groupedUnassigned.computeIfAbsent(keyExtent.tableId(), p -> new HashMap<>()).put(keyExtent,
- tServerInstance);
- });
+ unassigned.forEach((ke, lastTserver) -> groupedUnassigned
+ .computeIfAbsent(ke.tableId(), k -> new HashMap<>()).put(ke, lastTserver));
Map<TableId,String> tableIdToTableName = createdTableNameMap(getTableOperations().tableIdMap());
diff --git a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/TableLoadBalancer.java b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/TableLoadBalancer.java
index d5008f9..a41bbbc 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/TableLoadBalancer.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/TableLoadBalancer.java
@@ -116,11 +116,8 @@ public class TableLoadBalancer extends TabletBalancer {
Map<KeyExtent,TServerInstance> unassigned, Map<KeyExtent,TServerInstance> assignments) {
// separate the unassigned into tables
Map<TableId,Map<KeyExtent,TServerInstance>> groupedUnassigned = new HashMap<>();
- unassigned.forEach((keyExtent, tServerInstance) -> {
- groupedUnassigned.computeIfAbsent(keyExtent.tableId(), p -> new HashMap<>()).put(keyExtent,
- tServerInstance);
- });
-
+ unassigned.forEach((ke, lastTserver) -> groupedUnassigned
+ .computeIfAbsent(ke.tableId(), k -> new HashMap<>()).put(ke, lastTserver));
for (Entry<TableId,Map<KeyExtent,TServerInstance>> e : groupedUnassigned.entrySet()) {
Map<KeyExtent,TServerInstance> newAssignments = new HashMap<>();
getBalancerForTable(e.getKey()).getAssignments(current, e.getValue(), newAssignments);
diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
index 016d558..01fc5aa 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
@@ -704,21 +704,14 @@ public class MetadataTableUtil {
getTabletEntries(SortedMap<Key,Value> tabletKeyValues, List<ColumnFQ> columns) {
TreeMap<Text,SortedMap<ColumnFQ,Value>> tabletEntries = new TreeMap<>();
- HashSet<ColumnFQ> colSet = null;
- if (columns != null) {
- colSet = new HashSet<>(columns);
- }
+ HashSet<ColumnFQ> colSet = columns == null ? null : new HashSet<>(columns);
- for (Entry<Key,Value> entry : tabletKeyValues.entrySet()) {
- ColumnFQ currentKey = new ColumnFQ(entry.getKey());
- if (columns != null && !colSet.contains(currentKey)) {
- continue;
+ tabletKeyValues.forEach((key, val) -> {
+ ColumnFQ currentKey = new ColumnFQ(key);
+ if (columns == null || colSet.contains(currentKey)) {
+ tabletEntries.computeIfAbsent(key.getRow(), k -> new TreeMap<>()).put(currentKey, val);
}
-
- Text row = entry.getKey().getRow();
-
- tabletEntries.computeIfAbsent(row, k -> new TreeMap<>()).put(currentKey, entry.getValue());
- }
+ });
return tabletEntries;
}
diff --git a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancerTest.java b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancerTest.java
index 03ec136..a98640f 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancerTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancerTest.java
@@ -271,14 +271,12 @@ public class DefaultLoadBalancerTest {
if (expectedCounts != null) {
for (FakeTServer server : servers.values()) {
Map<String,Integer> counts = new HashMap<>();
- for (var extent : server.extents) {
+ server.extents.forEach(extent -> {
String t = extent.tableId().canonical();
counts.putIfAbsent(t, 0);
counts.put(t, counts.get(t) + 1);
- }
- for (var entry : counts.entrySet()) {
- assertEquals(expectedCounts.get(entry.getKey()), counts.get(entry.getKey()));
- }
+ });
+ counts.forEach((k, v) -> assertEquals(expectedCounts.get(k), v));
}
}
}
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java
index 0f270a6..159a3eb 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java
@@ -215,10 +215,6 @@ public class FileManager {
return ret;
}
- private static <T> List<T> getFileList(String file, Map<String,List<T>> files) {
- return files.computeIfAbsent(file, k -> new ArrayList<>());
- }
-
private void closeReaders(Collection<FileSKVIterator> filesToClose) {
for (FileSKVIterator reader : filesToClose) {
try {
@@ -372,7 +368,8 @@ public class FileManager {
for (FileSKVIterator reader : readers) {
String fileName = reservedReaders.remove(reader);
if (!sawIOException)
- getFileList(fileName, openFiles).add(new OpenReader(fileName, reader));
+ openFiles.computeIfAbsent(fileName, k -> new ArrayList<>())
+ .add(new OpenReader(fileName, reader));
}
}
@@ -571,9 +568,8 @@ public class FileManager {
List<String> files = dataSources.stream().map(x -> x.file).collect(Collectors.toList());
Map<FileSKVIterator,String> newlyReservedReaders = openFiles(files);
Map<String,List<FileSKVIterator>> map = new HashMap<>();
- newlyReservedReaders.forEach((reader, fileName) -> {
- map.computeIfAbsent(fileName, k -> new LinkedList<>()).add(reader);
- });
+ newlyReservedReaders.forEach(
+ (reader, fileName) -> map.computeIfAbsent(fileName, k -> new LinkedList<>()).add(reader));
for (FileDataSource fds : dataSources) {
FileSKVIterator source = map.get(fds.file).remove(0);