You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "ttttttz (Code Review)" <ge...@cloudera.org> on 2023/04/27 03:13:33 UTC

[Impala-ASF-CR] IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

ttttttz has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19810


Change subject: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
......................................................................

IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

During the processing of JNI Exceptions, some local references
were not released in a timely manner, which may lead to memory
leaks in the JVM.

Testing:

Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
---
M be/src/util/jni-util.cc
1 file changed, 4 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/19810/1
-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 1
Gerrit-Owner: ttttttz <24...@qq.com>

[Impala-ASF-CR] IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12877/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 2
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 07:48:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "ttttttz (Code Review)" <ge...@cloudera.org>.
ttttttz has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19810/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19810/3//COMMIT_MSG@14
PS3, Line 14: - Passed validation in the local development environment.
> If you could manually verify that the mem leak doesn't occur, it would be c
Ack



-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 3
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Wed, 03 May 2023 11:06:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................

IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

During the processing of JNI Exceptions, some local references
were not released in a timely manner, which may lead to memory
leaks in the JVM.

Testing:
- Manually verified that the memory leak doesn't occur in the
  local development environment.

Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Reviewed-on: http://gerrit.cloudera.org:8080/19810
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/util/jni-util.cc
1 file changed, 4 insertions(+), 1 deletion(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 6
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>

[Impala-ASF-CR] IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
......................................................................


Patch Set 2:

(3 comments)

Thanks for your work. I'm not a JNI expert but here are some comments.

http://gerrit.cloudera.org:8080/#/c/19810/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19810/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
This would be a less verbose title: "Avoid memory leaks in the handling of JNI exceptions".


http://gerrit.cloudera.org:8080/#/c/19810/2/be/src/util/jni-util.cc
File be/src/util/jni-util.cc:

http://gerrit.cloudera.org:8080/#/c/19810/2/be/src/util/jni-util.cc@292
PS2, Line 292: auto
Using the actual type is more readable. In this case it should be const char*.


http://gerrit.cloudera.org:8080/#/c/19810/2/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/19810/2/tests/query_test/test_udfs.py@695
PS2, Line 695:   def test_throws_exception(self, vector, unique_database):
How does this test that we don't leak memory?



-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 2
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 16:04:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "ttttttz (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Daniel Becker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19810

to look at the new patch set (#4).

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................

IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

During the processing of JNI Exceptions, some local references
were not released in a timely manner, which may lead to memory
leaks in the JVM.

Testing:
- Manually verified that the memory leak doesn't occur in the
  local development environment.

Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
---
M be/src/util/jni-util.cc
1 file changed, 4 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/19810/4
-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 4
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "ttttttz (Code Review)" <ge...@cloudera.org>.
ttttttz has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................


Patch Set 3:

> Patch Set 2:
> 
> (1 comment)

Hi Daniel Becker, thank you for your advice!. I pushed the revised code, please review it for me.


-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 3
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 10:26:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 5
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Thu, 04 May 2023 02:57:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19810/2/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/19810/2/tests/query_test/test_udfs.py@695
PS2, Line 695:   def test_throws_exception(self, vector, unique_database):
> Ack
I'm not sure how we could verify that there's no leak in a test.. Could you manually verify that the leak doesn't happen after this change? If so, you could write in the commit message in the Testing section that you manually verified that the problem doesn't occur.



-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 2
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 09:39:54 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................


Patch Set 4: Code-Review+2

LGTM. Nice catch!


-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 4
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Thu, 04 May 2023 02:56:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12889/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 3
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 10:40:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9280/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 5
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Thu, 04 May 2023 02:57:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12876/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 1
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 03:34:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

Posted by "ttttttz (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19810

to look at the new patch set (#2).

Change subject: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
......................................................................

IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

During the processing of JNI Exceptions, some local references
were not released in a timely manner, which may lead to memory
leaks in the JVM.

Testing:
- Run test_udfs

Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
---
M be/src/util/jni-util.cc
M tests/query_test/test_udfs.py
2 files changed, 18 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/19810/2
-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 2
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "ttttttz (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Daniel Becker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19810

to look at the new patch set (#3).

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................

IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

During the processing of JNI Exceptions, some local references
were not released in a timely manner, which may lead to memory
leaks in the JVM.

Testing:
- Passed validation in the local development environment.

Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
---
M be/src/util/jni-util.cc
1 file changed, 4 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/19810/3
-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 3
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

Thanks, it looks good, I left just one comment. Also, let's wait to see if Quanlong can review it.

http://gerrit.cloudera.org:8080/#/c/19810/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19810/3//COMMIT_MSG@14
PS3, Line 14: - Passed validation in the local development environment.
If you could manually verify that the mem leak doesn't occur, it would be clearer to write that, for example "Manually verified that the memory leak doesn't occur."



-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 3
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Wed, 03 May 2023 09:37:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: Avoid memory leaks in the handling of JNI exceptions
......................................................................


Patch Set 5: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 5
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Thu, 04 May 2023 08:13:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

Posted by "ttttttz (Code Review)" <ge...@cloudera.org>.
ttttttz has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19810/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19810/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
> This would be a less verbose title: "Avoid memory leaks in the handling of 
Ack


http://gerrit.cloudera.org:8080/#/c/19810/2/be/src/util/jni-util.cc
File be/src/util/jni-util.cc:

http://gerrit.cloudera.org:8080/#/c/19810/2/be/src/util/jni-util.cc@292
PS2, Line 292: auto
> Using the actual type is more readable. In this case it should be const cha
Ack


http://gerrit.cloudera.org:8080/#/c/19810/2/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/19810/2/tests/query_test/test_udfs.py@695
PS2, Line 695:   def test_throws_exception(self, vector, unique_database):
> How does this test that we don't leak memory?
Ack
Yes, the test case should have been written better. Currently, a memory leak is triggered when throws_exception() udf is run a certain number of times. This amount is related to the configured size of the JVM heap. Do you have any ideas on how to better detect memory leaks?



-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 2
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 03:11:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

Posted by "ttttttz (Code Review)" <ge...@cloudera.org>.
ttttttz has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
......................................................................


Patch Set 2:

> Patch Set 2:
> 
> (1 comment)
Thank you for your reply! It can be replicated by the steps in IMPALA-12101. I have no problem verifying locally.


-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 2
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 09:55:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown

Posted by "ttttttz (Code Review)" <ge...@cloudera.org>.
ttttttz has posted comments on this change. ( http://gerrit.cloudera.org:8080/19810 )

Change subject: IMPALA-12102: This patch avoids memory leaks due to the large number of JNI exceptions thrown
......................................................................


Patch Set 2:

Thank you for your reply! It can be replicated by the steps in IMPALA-12101. I have no problem verifying locally.


-- 
To view, visit http://gerrit.cloudera.org:8080/19810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4843df07dd0f9d3dc237f91db4ec00721ebbd680
Gerrit-Change-Number: 19810
Gerrit-PatchSet: 2
Gerrit-Owner: ttttttz <24...@qq.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: ttttttz <24...@qq.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 09:55:32 +0000
Gerrit-HasComments: No