maytasm commented on a change in pull request #11123:
URL: https://github.com/apache/druid/pull/11123#discussion_r628541779



##########
File path: 
server/src/main/java/org/apache/druid/segment/realtime/appenderator/AppenderatorImpl.java
##########
@@ -164,13 +168,24 @@
   private volatile Throwable persistError;
 
   private final boolean isRealTime;
+  /**
+   * Use next Map to store metadata (File, SegmentId) for a hydrant for batch 
appenderator
+   * in order to facilitate the mapping of the QueryableIndex associated with 
a given hydrant
+   * at merge time. This is necessary since batch appenderator will not map 
the QueryableIndex
+   * at persist time in order to minimize its memory footprint. This has to be 
synchronized since the
+   * map bay be accessed from multiple threads.

Review comment:
       typo

##########
File path: 
server/src/main/java/org/apache/druid/segment/realtime/appenderator/AppenderatorImpl.java
##########
@@ -691,6 +739,8 @@ public Object call() throws IOException
       if (sink.finishWriting()) {
         totalRows.addAndGet(-sink.getNumRows());
       }
+      // count hydrants for stats:
+      pushedHydrantsCount.addAndGet(IterableUtils.size(sink));

Review comment:
       You can use Iterables.size instead so you don't have to add new 
dependency on commons-collections4

##########
File path: docs/configuration/index.md
##########
@@ -1320,6 +1320,7 @@ Additional peon configs include:
 |`druid.peon.mode`|Choices are "local" and "remote". Setting this to local 
means you intend to run the peon as a standalone process (Not 
recommended).|remote|
 |`druid.indexer.task.baseDir`|Base temporary working 
directory.|`System.getProperty("java.io.tmpdir")`|
 |`druid.indexer.task.baseTaskDir`|Base temporary working directory for 
tasks.|`${druid.indexer.task.baseDir}/persistent/task`|
+|`druid.indexer.task.batchMemoryMappedIndex`|If false, native batch ingestion 
will not map indexes thus saving heap space. This does not apply to streaming 
ingestion, just to batch.|`false`|

Review comment:
       Can you add comment on how is this used, what's its purpose, and why 
would a user need to set it to true?

##########
File path: server/pom.xml
##########
@@ -454,6 +454,11 @@
             <version>1.3</version>
             <scope>test</scope>
         </dependency>
+        <dependency>

Review comment:
       Not needed. If we use Iterables.size instead




-- 
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org

Reply via email to