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 <ctubb...@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);