accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ctubb...@apache.org
Subject [accumulo] branch main updated: Clean up a few forEach loops (#1705)
Date Wed, 16 Sep 2020 00:09:56 GMT
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 <ctubbsii@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);


Mime
View raw message