You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "davisusanibar (via GitHub)" <gi...@apache.org> on 2023/04/25 00:11:35 UTC

[GitHub] [arrow] davisusanibar opened a new pull request, #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

davisusanibar opened a new pull request, #35314:
URL: https://github.com/apache/arrow/pull/35314

   <!--
   Thanks for opening a pull request!
   If this is your first pull request you can find detailed information on how 
   to contribute here:
     * [New Contributor's Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
     * [Contributing Overview](https://arrow.apache.org/docs/dev/developers/overview.html)
   
   
   If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
   
   Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
   
   Then could you also rename the pull request title in the following format?
   
       GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   or
   
       MINOR: [${COMPONENT}] ${SUMMARY}
   
   In the case of PARQUET issues on JIRA the title also supports:
   
       PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   -->
   
   ### Rationale for this change
   
   To close https://github.com/apache/arrow/issues/34723
   
   ### What changes are included in this PR?
   
   Enable log trace for Netty allocator memory usage 
   
   ### Are these changes tested?
   
   New log enabled:
   
   ```xml
   <configuration>
     <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
       <encoder>
         <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
       </encoder>
     </appender>
   
     <logger name="org.apache.arrow" additivity="false">
       <level value="trace" />
       <appender-ref ref="STDOUT" />
     </logger>
   
     <root level="trace">
       <appender-ref ref="STDOUT" />
     </root>
   
   </configuration>
   ```
   
   ```java
   try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
       1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
     buf.memoryAddress();
   }
   ```
   
   Logs:
   
   ```
   18:59:33.109 [allocation.logger] TRACE arrow.allocator - Memory Usage: 
   32 direct arena(s):
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Chunk(s) at 0~25%:
   none
   Chunk(s) at 0~50%:
   none
   Chunk(s) at 25~75%:
   none
   Chunk(s) at 50~100%:
   none
   Chunk(s) at 75~100%:
   none
   Chunk(s) at 100%:
   none
   small subpages:
   Large buffers outstanding: 0 totaling 0 bytes.
   Normal buffers outstanding: 0 totaling 0 bytes.
   ```
   
   ### Are there any user-facing changes?
   
   No


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1228471565


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,43 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    try {
+      logger.setLevel(Level.TRACE);
+      logger.addAppender(memoryLogsAppender);
+      memoryLogsAppender.start();
+      try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+          1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+        buf.memoryAddress();
+      }
+      BiFunction<List<String>, Level, Boolean> listAppenderContains =
+          (List<String> values, Level level) -> memoryLogsAppender.list.stream()
+              .anyMatch(
+                  log -> log.toString().contains(values.get(0)) &&
+                      log.toString().contains(values.get(1)) &&
+                      log.toString().contains(values.get(2)) &&
+                      log.getLevel().equals(level)
+              );
+      boolean result = false;
+      long startTime = System.currentTimeMillis();
+      while (true && (System.currentTimeMillis() - startTime) < 20000) {
+        result = listAppenderContains.apply(Arrays.asList("Memory Usage: \n", "Large buffers outstanding: ",

Review Comment:
   Changed



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1214633128


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1098,40 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    MemoryLogsAppender memoryLogsAppender = new MemoryLogsAppender();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();

Review Comment:
   Added



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1214633932


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1098,40 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    MemoryLogsAppender memoryLogsAppender = new MemoryLogsAppender();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();
+
+    try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+        1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+      buf.memoryAddress();
+    }
+
+    boolean contains = memoryLogsAppender
+        .contains(
+            Arrays.asList("Memory Usage: \n", "Large buffers outstanding: ", "Normal buffers outstanding: "),
+            Level.TRACE
+        );
+
+    assertTrue(contains);
+  }
+
+  class MemoryLogsAppender extends ListAppender<ILoggingEvent> {
+    public boolean contains(List<String> values, Level level) {
+      return list.stream()

Review Comment:
   Added



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] lidavidm commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1185309414


##########
java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java:
##########
@@ -265,7 +265,7 @@ public MemoryStatusThread() {
       @Override
       public void run() {
         while (true) {
-          memoryLogger.trace("Memory Usage: \n{}", PooledByteBufAllocatorL.this.toString());
+          memoryLogger.trace("Memory Usage: \n{}", PooledByteBufAllocatorL.this.allocator);

Review Comment:
   I'm just saying that the log message is generated on a background thread. So the test needs to somehow wait for the thread to actually run.
   
   Frankly: I don't think the test is really necessary for this.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #35314:
URL: https://github.com/apache/arrow/pull/35314#issuecomment-1594189548

   Conbench analyzed the 7 benchmark runs on commit `e4274c6e`.
   
   There were no benchmark performance regressions. 🎉
   
   The [full Conbench report](https://github.com/apache/arrow/runs/14313209051) has more details.


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1176423765


##########
java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java:
##########
@@ -265,7 +265,7 @@ public MemoryStatusThread() {
       @Override
       public void run() {
         while (true) {
-          memoryLogger.trace("Memory Usage: \n{}", PooledByteBufAllocatorL.this.toString());
+          memoryLogger.trace("Memory Usage: \n{}", PooledByteBufAllocatorL.this.allocator);

Review Comment:
   > Since this is a background thread, doesn't the test need to account for a potential race condition?
   
   Oh, please could you help me how/when this method execute/run? Or Could you help me with an example about the current race condition problem? This will help me to understand in more detail how this run behind the scene.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] lidavidm commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1175905492


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1098,40 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    MemoryLogsAppender memoryLogsAppender = new MemoryLogsAppender();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();
+
+    try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+        1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+      buf.memoryAddress();
+    }
+
+    boolean contains = memoryLogsAppender
+        .contains(
+            Arrays.asList("Memory Usage: \n", "Large buffers outstanding: ", "Normal buffers outstanding: "),
+            Level.TRACE
+        );
+
+    assertTrue(contains);
+  }
+
+  class MemoryLogsAppender extends ListAppender<ILoggingEvent> {
+    public boolean contains(List<String> values, Level level) {
+      return list.stream()

Review Comment:
   Just inline this? I don't see why we need a subclass and a method



##########
java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java:
##########
@@ -265,7 +265,7 @@ public MemoryStatusThread() {
       @Override
       public void run() {
         while (true) {
-          memoryLogger.trace("Memory Usage: \n{}", PooledByteBufAllocatorL.this.toString());
+          memoryLogger.trace("Memory Usage: \n{}", PooledByteBufAllocatorL.this.allocator);

Review Comment:
   Since this is a background thread, doesn't the test need to account for a potential race condition?



##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1098,40 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    MemoryLogsAppender memoryLogsAppender = new MemoryLogsAppender();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();

Review Comment:
   This test will permanently add an appender to a global logger, can we remove the appender afterwards?



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1227565120


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,32 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();
+    try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+        1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+      buf.memoryAddress();

Review Comment:
   Validation inside a loop



##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,32 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);

Review Comment:
   Added



##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,32 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();
+    try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+        1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+      buf.memoryAddress();
+    }
+    BiFunction<List<String>, Level, Boolean> listAppenderContains =
+        (List<String> values, Level level) -> memoryLogsAppender.list.stream()
+            .anyMatch(
+                log -> log.toString().contains(values.get(0)) &&
+                    log.toString().contains(values.get(1)) &&
+                    log.toString().contains(values.get(2)) &&
+                    log.getLevel().equals(level)
+            );
+    boolean result = listAppenderContains.apply(Arrays.asList("Memory Usage: \n", "Large buffers outstanding: ",
+            "Normal buffers outstanding: "),
+        Level.TRACE);

Review Comment:
   Added



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1227565313


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,32 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();
+    try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+        1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+      buf.memoryAddress();
+    }
+    BiFunction<List<String>, Level, Boolean> listAppenderContains =
+        (List<String> values, Level level) -> memoryLogsAppender.list.stream()
+            .anyMatch(
+                log -> log.toString().contains(values.get(0)) &&
+                    log.toString().contains(values.get(1)) &&
+                    log.toString().contains(values.get(2)) &&
+                    log.getLevel().equals(level)
+            );
+    boolean result = listAppenderContains.apply(Arrays.asList("Memory Usage: \n", "Large buffers outstanding: ",
+            "Normal buffers outstanding: "),
+        Level.TRACE);
+    logger.detachAppender(memoryLogsAppender);

Review Comment:
   Added



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1214633769


##########
java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java:
##########
@@ -265,7 +265,7 @@ public MemoryStatusThread() {
       @Override
       public void run() {
         while (true) {
-          memoryLogger.trace("Memory Usage: \n{}", PooledByteBufAllocatorL.this.toString());
+          memoryLogger.trace("Memory Usage: \n{}", PooledByteBufAllocatorL.this.allocator);

Review Comment:
   Ok let's continue as it is now.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] lidavidm commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1214637277


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,32 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();
+    try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+        1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+      buf.memoryAddress();
+    }
+    BiFunction<List<String>, Level, Boolean> listAppenderContains =
+        (List<String> values, Level level) -> memoryLogsAppender.list.stream()
+            .anyMatch(
+                log -> log.toString().contains(values.get(0)) &&
+                    log.toString().contains(values.get(1)) &&
+                    log.toString().contains(values.get(2)) &&
+                    log.getLevel().equals(level)
+            );
+    boolean result = listAppenderContains.apply(Arrays.asList("Memory Usage: \n", "Large buffers outstanding: ",
+            "Normal buffers outstanding: "),
+        Level.TRACE);

Review Comment:
   Just write it as a loop...



##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,32 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();
+    try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+        1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+      buf.memoryAddress();
+    }
+    BiFunction<List<String>, Level, Boolean> listAppenderContains =
+        (List<String> values, Level level) -> memoryLogsAppender.list.stream()
+            .anyMatch(
+                log -> log.toString().contains(values.get(0)) &&
+                    log.toString().contains(values.get(1)) &&
+                    log.toString().contains(values.get(2)) &&
+                    log.getLevel().equals(level)
+            );
+    boolean result = listAppenderContains.apply(Arrays.asList("Memory Usage: \n", "Large buffers outstanding: ",
+            "Normal buffers outstanding: "),
+        Level.TRACE);
+    logger.detachAppender(memoryLogsAppender);

Review Comment:
   this needs to be in a try-finally



##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,32 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);

Review Comment:
   Reset the log level afterwards, too



##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,32 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();
+    try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+        1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+      buf.memoryAddress();

Review Comment:
   And IMO, the easier way to test this would be to factor out a function that generates the log string, test that function, and use that function from the thread, rather than hooking into the physical logger.



##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,32 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    logger.setLevel(Level.TRACE);
+    logger.addAppender(memoryLogsAppender);
+    memoryLogsAppender.start();
+    try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+        1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+      buf.memoryAddress();

Review Comment:
   What I'm saying is, you have no guarantee the logger will actually have logged anything by the time the next few lines run. So this is just adding a flaky test.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] lidavidm commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1228543844


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1097,40 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    try {
+      logger.setLevel(Level.TRACE);
+      logger.addAppender(memoryLogsAppender);
+      memoryLogsAppender.start();
+      try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+          1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+        buf.memoryAddress();
+      }
+      boolean result = false;
+      long startTime = System.currentTimeMillis();
+      while (true) {
+        result = memoryLogsAppender.list.stream()
+            .anyMatch(
+                log -> log.toString().contains("Memory Usage: \n") &&
+                    log.toString().contains("Large buffers outstanding: ") &&
+                    log.toString().contains("Normal buffers outstanding: ") &&
+                    log.getLevel().equals(Level.TRACE)
+            );
+        if (result || (System.currentTimeMillis() - startTime) > 10000) { // 10 seconds maximum for time to write logs

Review Comment:
   you could just make this the loop guard?



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35314:
URL: https://github.com/apache/arrow/pull/35314#issuecomment-1520979921

   * Closes: #34723


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1228775559


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1097,40 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    try {
+      logger.setLevel(Level.TRACE);
+      logger.addAppender(memoryLogsAppender);
+      memoryLogsAppender.start();
+      try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+          1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+        buf.memoryAddress();
+      }
+      boolean result = false;
+      long startTime = System.currentTimeMillis();
+      while (true) {
+        result = memoryLogsAppender.list.stream()
+            .anyMatch(
+                log -> log.toString().contains("Memory Usage: \n") &&
+                    log.toString().contains("Large buffers outstanding: ") &&
+                    log.toString().contains("Normal buffers outstanding: ") &&
+                    log.getLevel().equals(Level.TRACE)
+            );
+        if (result || (System.currentTimeMillis() - startTime) > 10000) { // 10 seconds maximum for time to write logs

Review Comment:
   Added



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] lidavidm commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1228010038


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,43 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    try {
+      logger.setLevel(Level.TRACE);
+      logger.addAppender(memoryLogsAppender);
+      memoryLogsAppender.start();
+      try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+          1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+        buf.memoryAddress();
+      }
+      BiFunction<List<String>, Level, Boolean> listAppenderContains =
+          (List<String> values, Level level) -> memoryLogsAppender.list.stream()
+              .anyMatch(
+                  log -> log.toString().contains(values.get(0)) &&
+                      log.toString().contains(values.get(1)) &&
+                      log.toString().contains(values.get(2)) &&
+                      log.getLevel().equals(level)
+              );
+      boolean result = false;
+      long startTime = System.currentTimeMillis();
+      while (true && (System.currentTimeMillis() - startTime) < 20000) {
+        result = listAppenderContains.apply(Arrays.asList("Memory Usage: \n", "Large buffers outstanding: ",

Review Comment:
   Again...why the lambda? Just write it out as a loop



##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,43 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    try {
+      logger.setLevel(Level.TRACE);
+      logger.addAppender(memoryLogsAppender);
+      memoryLogsAppender.start();
+      try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+          1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+        buf.memoryAddress();
+      }
+      BiFunction<List<String>, Level, Boolean> listAppenderContains =
+          (List<String> values, Level level) -> memoryLogsAppender.list.stream()
+              .anyMatch(
+                  log -> log.toString().contains(values.get(0)) &&
+                      log.toString().contains(values.get(1)) &&
+                      log.toString().contains(values.get(2)) &&
+                      log.getLevel().equals(level)
+              );
+      boolean result = false;
+      long startTime = System.currentTimeMillis();
+      while (true && (System.currentTimeMillis() - startTime) < 20000) {
+        result = listAppenderContains.apply(Arrays.asList("Memory Usage: \n", "Large buffers outstanding: ",
+                "Normal buffers outstanding: "), Level.TRACE);
+        if (result) {
+          break;
+        }
+      }
+      memoryLogsAppender.stop();
+      assertTrue(result);
+    } finally {
+      logger.detachAppender(memoryLogsAppender);
+      logger.setLevel(null);

Review Comment:
   Shouldn't we reset it to what it originally was?



##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,43 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    try {
+      logger.setLevel(Level.TRACE);
+      logger.addAppender(memoryLogsAppender);
+      memoryLogsAppender.start();
+      try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+          1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+        buf.memoryAddress();
+      }
+      BiFunction<List<String>, Level, Boolean> listAppenderContains =
+          (List<String> values, Level level) -> memoryLogsAppender.list.stream()
+              .anyMatch(
+                  log -> log.toString().contains(values.get(0)) &&
+                      log.toString().contains(values.get(1)) &&
+                      log.toString().contains(values.get(2)) &&
+                      log.getLevel().equals(level)
+              );
+      boolean result = false;
+      long startTime = System.currentTimeMillis();
+      while (true && (System.currentTimeMillis() - startTime) < 20000) {

Review Comment:
   Why the redundant condition?



##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,43 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    try {
+      logger.setLevel(Level.TRACE);
+      logger.addAppender(memoryLogsAppender);
+      memoryLogsAppender.start();
+      try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+          1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+        buf.memoryAddress();

Review Comment:
   Why can't we just use an allocator to create the buffer?



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35314:
URL: https://github.com/apache/arrow/pull/35314#issuecomment-1520979948

   :warning: GitHub issue #34723 **has been automatically assigned in GitHub** to PR creator.


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1228472598


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,43 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    try {
+      logger.setLevel(Level.TRACE);
+      logger.addAppender(memoryLogsAppender);
+      memoryLogsAppender.start();
+      try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+          1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+        buf.memoryAddress();
+      }
+      BiFunction<List<String>, Level, Boolean> listAppenderContains =
+          (List<String> values, Level level) -> memoryLogsAppender.list.stream()
+              .anyMatch(
+                  log -> log.toString().contains(values.get(0)) &&
+                      log.toString().contains(values.get(1)) &&
+                      log.toString().contains(values.get(2)) &&
+                      log.getLevel().equals(level)
+              );
+      boolean result = false;
+      long startTime = System.currentTimeMillis();
+      while (true && (System.currentTimeMillis() - startTime) < 20000) {
+        result = listAppenderContains.apply(Arrays.asList("Memory Usage: \n", "Large buffers outstanding: ",
+                "Normal buffers outstanding: "), Level.TRACE);
+        if (result) {
+          break;
+        }
+      }
+      memoryLogsAppender.stop();
+      assertTrue(result);
+    } finally {
+      logger.detachAppender(memoryLogsAppender);
+      logger.setLevel(null);

Review Comment:
   Initially it was marked with null



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1228472332


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,43 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    try {
+      logger.setLevel(Level.TRACE);
+      logger.addAppender(memoryLogsAppender);
+      memoryLogsAppender.start();
+      try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+          1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+        buf.memoryAddress();
+      }
+      BiFunction<List<String>, Level, Boolean> listAppenderContains =
+          (List<String> values, Level level) -> memoryLogsAppender.list.stream()
+              .anyMatch(
+                  log -> log.toString().contains(values.get(0)) &&
+                      log.toString().contains(values.get(1)) &&
+                      log.toString().contains(values.get(2)) &&
+                      log.getLevel().equals(level)
+              );
+      boolean result = false;
+      long startTime = System.currentTimeMillis();
+      while (true && (System.currentTimeMillis() - startTime) < 20000) {

Review Comment:
   Moved inside de loop to try to read logs for 10 seconds maximum, then finished with error.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] davisusanibar commented on a diff in pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #35314:
URL: https://github.com/apache/arrow/pull/35314#discussion_r1228473853


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -1091,6 +1099,43 @@ public void testMemoryLeakWithReservation() throws Exception {
     }
   }
 
+  @Test
+  public void testMemoryUsage() {
+    ListAppender<ILoggingEvent> memoryLogsAppender = new ListAppender<>();
+    Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator");
+    try {
+      logger.setLevel(Level.TRACE);
+      logger.addAppender(memoryLogsAppender);
+      memoryLogsAppender.start();
+      try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null,
+          1024, new PooledByteBufAllocatorL().empty.memoryAddress())) {
+        buf.memoryAddress();

Review Comment:
   Just trying to use allocator but for some reason the log is not populated if I run the test by command line, but able to populate log if I run the same test with allocator thru the IDE.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] lidavidm merged pull request #35314: GH-34723: [Java] Enable log trace for Netty allocator memory usage

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #35314:
URL: https://github.com/apache/arrow/pull/35314


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org