HIVE-24179: Memory leak in HS2 DbTxnManager when compiling SHOW LOCKS statement #1509
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deniskuzZ
reviewed
Sep 21, 2020
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/lock/show/ShowDbLocksAnalyzer.java
Outdated
Show resolved
Hide resolved
kgyrtkirk
added
tests pending
tests unstable
tests passed
and removed
tests passed
tests pending
tests unstable
labels
deniskuzZ
approved these changes
Sep 22, 2020
… statement 1. Avoid multiple shutdown hooks for the same threadpool in DbTxnManager. Enforce unique initialization and shutdown of the thread pool. Apart from the apparent memory leak there is no need to register a shutdown hook for the executor service multiple times. It wastes memory, complicates code, and serves no purpose. Change initHeartbeatExecutorService to synchronized putting the class lock in the whole method. Mixing instance and class locks is confusing and does not offer any particular benefit. 2. Avoid HiveTxnManager instantiation in lock analyzers. A transaction manager should already exist inside the analyzer before calling the analyze method. There is no need to create a new one just to obtain the useNewShowLocksFormat boolean indicator.
szehon-ho
pushed a commit
to szehon-ho/hive
that referenced
this pull request
Nov 25, 2020
… statement (Stamatis Zampetakis, reviewed by Denys Kuzmenko, Jesus Camacho Rodriguez) 1. Avoid multiple shutdown hooks for the same threadpool in DbTxnManager. Enforce unique initialization and shutdown of the thread pool. Apart from the apparent memory leak there is no need to register a shutdown hook for the executor service multiple times. It wastes memory, complicates code, and serves no purpose. Change initHeartbeatExecutorService to synchronized putting the class lock in the whole method. Mixing instance and class locks is confusing and does not offer any particular benefit. 2. Avoid HiveTxnManager instantiation in lock analyzers. A transaction manager should already exist inside the analyzer before calling the analyze method. There is no need to create a new one just to obtain the useNewShowLocksFormat boolean indicator. Closes apache#1509
mr3project
pushed a commit
to mr3project/hive-mr3
that referenced
this pull request
Sep 24, 2023
… statement (Stamatis Zampetakis, reviewed by Denys Kuzmenko, Jesus Camacho Rodriguez) 1. Avoid multiple shutdown hooks for the same threadpool in DbTxnManager. Enforce unique initialization and shutdown of the thread pool. Apart from the apparent memory leak there is no need to register a shutdown hook for the executor service multiple times. It wastes memory, complicates code, and serves no purpose. Change initHeartbeatExecutorService to synchronized putting the class lock in the whole method. Mixing instance and class locks is confusing and does not offer any particular benefit. 2. Avoid HiveTxnManager instantiation in lock analyzers. A transaction manager should already exist inside the analyzer before calling the analyze method. There is no need to create a new one just to obtain the useNewShowLocksFormat boolean indicator. Closes apache/hive#1509
mr3project
pushed a commit
to mr3project/hive-mr3
that referenced
this pull request
Oct 2, 2023
… statement (Stamatis Zampetakis, reviewed by Denys Kuzmenko, Jesus Camacho Rodriguez) 1. Avoid multiple shutdown hooks for the same threadpool in DbTxnManager. Enforce unique initialization and shutdown of the thread pool. Apart from the apparent memory leak there is no need to register a shutdown hook for the executor service multiple times. It wastes memory, complicates code, and serves no purpose. Change initHeartbeatExecutorService to synchronized putting the class lock in the whole method. Mixing instance and class locks is confusing and does not offer any particular benefit. 2. Avoid HiveTxnManager instantiation in lock analyzers. A transaction manager should already exist inside the analyzer before calling the analyze method. There is no need to create a new one just to obtain the useNewShowLocksFormat boolean indicator. Closes apache/hive#1509
mr3project
pushed a commit
to mr3project/hive-mr3
that referenced
this pull request
Nov 7, 2023
… statement (Stamatis Zampetakis, reviewed by Denys Kuzmenko, Jesus Camacho Rodriguez) 1. Avoid multiple shutdown hooks for the same threadpool in DbTxnManager. Enforce unique initialization and shutdown of the thread pool. Apart from the apparent memory leak there is no need to register a shutdown hook for the executor service multiple times. It wastes memory, complicates code, and serves no purpose. Change initHeartbeatExecutorService to synchronized putting the class lock in the whole method. Mixing instance and class locks is confusing and does not offer any particular benefit. 2. Avoid HiveTxnManager instantiation in lock analyzers. A transaction manager should already exist inside the analyzer before calling the analyze method. There is no need to create a new one just to obtain the useNewShowLocksFormat boolean indicator. Closes apache/hive#1509
mr3project
pushed a commit
to mr3project/hive-mr3
that referenced
this pull request
Dec 4, 2023
… statement (Stamatis Zampetakis, reviewed by Denys Kuzmenko, Jesus Camacho Rodriguez) 1. Avoid multiple shutdown hooks for the same threadpool in DbTxnManager. Enforce unique initialization and shutdown of the thread pool. Apart from the apparent memory leak there is no need to register a shutdown hook for the executor service multiple times. It wastes memory, complicates code, and serves no purpose. Change initHeartbeatExecutorService to synchronized putting the class lock in the whole method. Mixing instance and class locks is confusing and does not offer any particular benefit. 2. Avoid HiveTxnManager instantiation in lock analyzers. A transaction manager should already exist inside the analyzer before calling the analyze method. There is no need to create a new one just to obtain the useNewShowLocksFormat boolean indicator. Closes apache/hive#1509
mr3project
pushed a commit
to mr3project/hive-mr3
that referenced
this pull request
Dec 9, 2023
… statement (Stamatis Zampetakis, reviewed by Denys Kuzmenko, Jesus Camacho Rodriguez) 1. Avoid multiple shutdown hooks for the same threadpool in DbTxnManager. Enforce unique initialization and shutdown of the thread pool. Apart from the apparent memory leak there is no need to register a shutdown hook for the executor service multiple times. It wastes memory, complicates code, and serves no purpose. Change initHeartbeatExecutorService to synchronized putting the class lock in the whole method. Mixing instance and class locks is confusing and does not offer any particular benefit. 2. Avoid HiveTxnManager instantiation in lock analyzers. A transaction manager should already exist inside the analyzer before calling the analyze method. There is no need to create a new one just to obtain the useNewShowLocksFormat boolean indicator. Closes apache/hive#1509
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Why are the changes needed?
Avoid the memory leak when executing
SHOW LOCKSstatements and simplify code.Does this PR introduce any user-facing change?
No
How was this patch tested?
mvn test -pl itests/hive-unit -Dtest=TestDbTxnManagerMemoryLeak -PitestsIf it finishes then there is no memory leak, otherwise most of the time you get an OutOfMemoryError.
See the JIRA case for more details.
The commit with the test is not meant to be merged into master