You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "rickyma (via GitHub)" <gi...@apache.org> on 2024/02/23 07:30:36 UTC

[PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

rickyma opened a new pull request, #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537

   ### What changes were proposed in this pull request?
   
   Memory may leak in exceptional scenarios when flushing events
   
   ### Why are the changes needed?
   
   A follow-up PR for: https://github.com/apache/incubator-uniffle/pull/1459
   
   I found out that memory may leak in exceptional scenarios when flushing events.
   <img width="560" alt="企业微信截图_17086201359387" src="https://github.com/apache/incubator-uniffle/assets/13834479/e7b77e82-4336-4123-b59a-621346edd613">
   <img width="811" alt="企业微信截图_1708620172540" src="https://github.com/apache/incubator-uniffle/assets/13834479/931eeb38-0c1b-4cfb-83be-485b74c10e17">
   
   After this PR, the memory will not leak and the exception's stack trace will be like:
   [2024-02-23 14:25:02.614] [LocalFileFlushEventThreadPool-54] [ERROR] DefaultFlushEventHandler.handleEventAndUpdateMetrics - Unexpected exceptions happened due to
   java.lang.NullPointerException
           at org.apache.uniffle.server.ShuffleTaskInfo.getMaxConcurrencyPerPartitionToWrite(ShuffleTaskInfo.java:109)
           at org.apache.uniffle.server.ShuffleFlushManager.getMaxConcurrencyPerPartitionWrite(ShuffleFlushManager.java:198)
           at org.apache.uniffle.server.ShuffleFlushManager.processFlushEvent(ShuffleFlushManager.java:149)
           at org.apache.uniffle.server.DefaultFlushEventHandler.handleEventAndUpdateMetrics(DefaultFlushEventHandler.java:88)
           at org.apache.uniffle.server.DefaultFlushEventHandler.lambda$dispatchEvent$0(DefaultFlushEventHandler.java:205)
           at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
           at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
           at java.lang.Thread.run(Thread.java:750)
   
   We can talk about the above exception in another issue after this PR is merged.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   Existing UTs.
   


-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#issuecomment-1960858776

   ## [Codecov](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1537?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: Patch coverage is `0%` with `2 lines` in your changes are missing coverage. Please review.
   > Project coverage is 54.66%. Comparing base [(`59aa30d`)](https://app.codecov.io/gh/apache/incubator-uniffle/commit/59aa30d3b153d9df8bc13e3e676c993292c1b861?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`dd8694b`)](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1537?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   
   | [Files](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...pache/uniffle/server/DefaultFlushEventHandler.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9EZWZhdWx0Rmx1c2hFdmVudEhhbmRsZXIuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1537      +/-   ##
   ============================================
   + Coverage     54.17%   54.66%   +0.48%     
   + Complexity     2822     2280     -542     
   ============================================
     Files           435      357      -78     
     Lines         24501    15880    -8621     
     Branches       2074     1444     -630     
   ============================================
   - Hits          13274     8681    -4593     
   + Misses        10397     6704    -3693     
   + Partials        830      495     -335     
   ```
   
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1537?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500629483


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;

Review Comment:
   This is because once the `ShuffleBuffer` associated with the `appId` expires, its corresponding `bufferPool` will be cleared by a scheduled task. Consequently, the `event` related to the `appId` will become `inValid`, and there is no need to repeatedly release memory, as the scheduled task has already taken care of releasing the buffer memory for all events associated with that `appId`. You can check the code for more detailed reasons. @EnricoMi 
   
   As for why the code is designed this way, you can ask @zuston 



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500787366


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;

Review Comment:
   @EnricoMi Your commit is merged.



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500787366


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;

Review Comment:
   @EnricoMi Your commit is merged. But spotless CI failed. You can refactor the code.



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#issuecomment-1960852364

   @zuston @jerqi PTAL.


-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500787366


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;

Review Comment:
   @EnricoMi Your commit is merged. But spotless IC failed. You can refactor the code.



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston merged PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537


-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1501369585


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;

Review Comment:
   > This is because once the ShuffleBuffer associated with the appId expires, its corresponding bufferPool will be cleared by a scheduled task. Consequently, the event related to the appId will become inValid, and there is no need to repeatedly release memory, as the scheduled task has already taken care of releasing the buffer memory for all events associated with that appId.
   
   Absolutely right.



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500369179


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;
       }
+
+      LOG.error(
+          "Unexpected exceptions happened when handling the flush event: {}, due to ", event, e);
+      // We need to release the memory when unexpected exceptions happened
+      event.doCleanup();

Review Comment:
   Should we put the logic to the code block finally?



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500787366


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;

Review Comment:
   @EnricoMi 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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500369179


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;
       }
+
+      LOG.error(
+          "Unexpected exceptions happened when handling the flush event: {}, due to ", event, e);
+      // We need to release the memory when unexpected exceptions happened
+      event.doCleanup();

Review Comment:
   Should we put the logic to the code block finally?



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "EnricoMi (via GitHub)" <gi...@apache.org>.
EnricoMi commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500476923


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;

Review Comment:
   Why wouldn't you release an invalid event?



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500629483


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;

Review Comment:
   This is because once the `ShuffleBuffer` associated with the `appId` expires, its corresponding `bufferPool` will be cleared by a scheduled task. Consequently, the `event` related to the `appId` will become invalid, and there is no need to repeatedly release memory, as the scheduled task has already taken care of releasing the buffer memory for all events associated with that `appId`. You can check the code for more detailed reasons. @EnricoMi 
   
   As for why the code is designed this way, you can ask @zuston 



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

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

   ## Test Results
   2 438 files  +1  2 438 suites  +1   4h 43m 5s :stopwatch: +45s
     823 tests ±0    822 :white_check_mark: +1   1 :zzz: ±0  0 :x: ±0 
   9 743 runs  +1  9 729 :white_check_mark: +2  14 :zzz: ±0  0 :x: ±0 
   
   Results for commit dd8694b2. ± Comparison against base commit 59aa30d3.
   
   [test-results]:data:application/gzip;base64,H4sIAPhO2GUC/03MTQ6DIBCG4asY1l3ICDr0Mg0wmJD60yCsTO9eMGhdfg/Du7PRT25jzwZEh4+GbcnH+6YUdPTrkoX3CmWm/B7LBUJ3rteWrD0I/vT2n/LrglH7KUN7gQthDVVCWkpTDaKr62yqAdRFtSlOuDWPfU/adZ59zIMRYa+EgRE5Idl2cFxKFE4S9cYAdyClAc3Z9wf74z8LEAEAAA==
   


-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500508670


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {

Review Comment:
   cc @zuston 



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [PR] [#1459][FOLLOWUP] fix(server): Memory may leak in exceptional scenarios when flushing events [incubator-uniffle]

Posted by "EnricoMi (via GitHub)" <gi...@apache.org>.
EnricoMi commented on code in PR #1537:
URL: https://github.com/apache/incubator-uniffle/pull/1537#discussion_r1500762019


##########
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java:
##########
@@ -126,6 +126,11 @@ private void handleEventAndUpdateMetrics(ShuffleDataFlushEvent event, Storage st
       if (e instanceof EventInvalidException) {
         return;

Review Comment:
   Reasonable. Maybe it is worth mentioning this fact here. Something like
   ```suggestion
           // invalid events have already been released / cleaned up so no need to call event.doCleanup() here
           return;
   ```



-- 
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: issues-unsubscribe@uniffle.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org