sashidhar commented on a change in pull request #7764: #7316 Use 
Map.putIfAbsent() instead of containsKey() + put()
URL: https://github.com/apache/incubator-druid/pull/7764#discussion_r287866951
 
 

 ##########
 File path: 
indexing-service/src/main/java/org/apache/druid/indexing/firehose/IngestSegmentFirehoseFactory.java
 ##########
 @@ -204,87 +204,87 @@ public Firehose connect(InputRowParser inputRowParser, 
File temporaryDirectory)
         segmentIds
     );
 
-    try {
-      final List<TimelineObjectHolder<String, DataSegment>> timeLineSegments = 
getTimeline();
-
-      // Download all segments locally.
-      // Note: this requires enough local storage space to fit all of the 
segments, even though
-      // IngestSegmentFirehose iterates over the segments in series. We may 
want to change this
-      // to download files lazily, perhaps sharing code with 
PrefetchableTextFilesFirehoseFactory.
-      final SegmentLoader segmentLoader = 
segmentLoaderFactory.manufacturate(temporaryDirectory);
-      Map<DataSegment, File> segmentFileMap = Maps.newLinkedHashMap();
-      for (TimelineObjectHolder<String, DataSegment> holder : 
timeLineSegments) {
-        for (PartitionChunk<DataSegment> chunk : holder.getObject()) {
-          final DataSegment segment = chunk.getObject();
-          if (!segmentFileMap.containsKey(segment)) {
-            segmentFileMap.put(segment, 
segmentLoader.getSegmentFiles(segment));
+    final List<TimelineObjectHolder<String, DataSegment>> timeLineSegments = 
getTimeline();
+
+    // Download all segments locally.
+    // Note: this requires enough local storage space to fit all of the 
segments, even though
+    // IngestSegmentFirehose iterates over the segments in series. We may want 
to change this
+    // to download files lazily, perhaps sharing code with 
PrefetchableTextFilesFirehoseFactory.
+    final SegmentLoader segmentLoader = 
segmentLoaderFactory.manufacturate(temporaryDirectory);
+    Map<DataSegment, File> segmentFileMap = Maps.newLinkedHashMap();
+    for (TimelineObjectHolder<String, DataSegment> holder : timeLineSegments) {
+      for (PartitionChunk<DataSegment> chunk : holder.getObject()) {
+        final DataSegment segment = chunk.getObject();
+
+        segmentFileMap.computeIfAbsent(segment, k -> {
+          try {
+            return segmentLoader.getSegmentFiles(segment);
           }
-        }
-      }
+          catch (SegmentLoadingException e) {
+            throw new RuntimeException(e);
+          }
+        });
 
-      final List<String> dims;
-      if (dimensions != null) {
-        dims = dimensions;
-      } else if 
(inputRowParser.getParseSpec().getDimensionsSpec().hasCustomDimensions()) {
-        dims = 
inputRowParser.getParseSpec().getDimensionsSpec().getDimensionNames();
-      } else {
-        dims = getUniqueDimensions(
-            timeLineSegments,
-            
inputRowParser.getParseSpec().getDimensionsSpec().getDimensionExclusions()
-        );
       }
+    }
 
-      final List<String> metricsList = metrics == null ? 
getUniqueMetrics(timeLineSegments) : metrics;
+    final List<String> dims;
+    if (dimensions != null) {
+      dims = dimensions;
+    } else if 
(inputRowParser.getParseSpec().getDimensionsSpec().hasCustomDimensions()) {
+      dims = 
inputRowParser.getParseSpec().getDimensionsSpec().getDimensionNames();
+    } else {
+      dims = getUniqueDimensions(
+        timeLineSegments,
+        
inputRowParser.getParseSpec().getDimensionsSpec().getDimensionExclusions()
+      );
+    }
 
-      final List<WindowedStorageAdapter> adapters = Lists.newArrayList(
-          Iterables.concat(
-              Iterables.transform(
-                  timeLineSegments,
-                  new Function<TimelineObjectHolder<String, DataSegment>, 
Iterable<WindowedStorageAdapter>>()
-                  {
+    final List<String> metricsList = metrics == null ? 
getUniqueMetrics(timeLineSegments) : metrics;
+
+    final List<WindowedStorageAdapter> adapters = Lists.newArrayList(
+        Iterables.concat(
+        Iterables.transform(
+          timeLineSegments,
+          new Function<TimelineObjectHolder<String, DataSegment>, 
Iterable<WindowedStorageAdapter>>() {
+            @Override
+            public Iterable<WindowedStorageAdapter> apply(final 
TimelineObjectHolder<String, DataSegment> holder)
+            {
+              return
+                Iterables.transform(
+                  holder.getObject(),
+                  new Function<PartitionChunk<DataSegment>, 
WindowedStorageAdapter>() {
                     @Override
-                    public Iterable<WindowedStorageAdapter> apply(final 
TimelineObjectHolder<String, DataSegment> holder)
+                    public WindowedStorageAdapter apply(final 
PartitionChunk<DataSegment> input)
                     {
-                      return
-                          Iterables.transform(
-                              holder.getObject(),
-                              new Function<PartitionChunk<DataSegment>, 
WindowedStorageAdapter>()
-                              {
-                                @Override
-                                public WindowedStorageAdapter apply(final 
PartitionChunk<DataSegment> input)
-                                {
-                                  final DataSegment segment = 
input.getObject();
-                                  try {
-                                    return new WindowedStorageAdapter(
-                                        new QueryableIndexStorageAdapter(
-                                            indexIO.loadIndex(
-                                                Preconditions.checkNotNull(
-                                                    
segmentFileMap.get(segment),
-                                                    "File for segment %s", 
segment.getId()
-                                                )
-                                            )
-                                        ),
-                                        holder.getInterval()
-                                    );
-                                  }
-                                  catch (IOException e) {
-                                    throw new RuntimeException(e);
-                                  }
-                                }
-                              }
-                          );
+                      final DataSegment segment = input.getObject();
+                      try {
+                        return new WindowedStorageAdapter(
+                          new QueryableIndexStorageAdapter(
+                            indexIO.loadIndex(
+                              Preconditions.checkNotNull(
+                                segmentFileMap.get(segment),
+                                "File for segment %s", segment.getId()
+                              )
+                            )
+                          ),
+                          holder.getInterval()
+                        );
+                      }
+                      catch (IOException e) {
+                        throw new RuntimeException(e);
+                      }
                     }
                   }
-              )
-          )
-      );
+                );
+            }
+          }
+        )
+      )
+    );
 
-      final TransformSpec transformSpec = 
TransformSpec.fromInputRowParser(inputRowParser);
-      return new IngestSegmentFirehose(adapters, transformSpec, dims, 
metricsList, dimFilter);
-    }
-    catch (SegmentLoadingException e) {
-      throw new RuntimeException(e);
-    }
 
 Review comment:
   Outer try/catch block removed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to