You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/08/11 17:30:08 UTC
[GitHub] [hive] zabetak opened a new pull request, #3514: HIVE-26404: HMS memory leak when compaction cleaner fails to remove obsolete files
zabetak opened a new pull request, #3514:
URL: https://github.com/apache/hive/pull/3514
### What changes were proposed in this pull request and why?
* Add unit test to trigger the memory leak (not meant to be committed in the repo cause it is a bit hacky and slow)
* (Main fix) Ensure FileSystem resources are closed correctly by wrapping the code in try-finally block.
In order for the issue to appear the following requirements must be met:
* Secure HMS/HDFS communication
* Many compaction failures when removing files
* Many users doas running compactions
More info under HIVE-26404.
### Does this PR introduce _any_ user-facing change?
Fixes the memory leak.
### How was this patch tested?
Run the new test with and without the fix.
Without the fix the test:
* it will either hang and there will be `OutOfMemory` in `target/surefire-reports/org.apache.hadoop.hive.ql.txn.compactor.TestCleanerWithSecureDFS-output.txt`
* it will finish after a long time and there will be `OutOfMemory` in `target/surefire-reports/org.apache.hadoop.hive.ql.txn.compactor.TestCleanerWithSecureDFS-output.txt`
To confirm the memory leak it is the same with the one obseved in HIVE-26404 I took a heapdump when the test was running and validated that there are many `FileSystem` objects in the cache each holding `Configuration` objects.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a diff in pull request #3514: HIVE-26404: HMS memory leak when compaction cleaner fails to remove obsolete files
Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3514:
URL: https://github.com/apache/hive/pull/3514#discussion_r975252543
##########
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCleanerWithReplication.java:
##########
@@ -44,34 +39,25 @@
import org.junit.Test;
import javax.security.auth.login.LoginException;
-import java.io.File;
import java.io.IOException;
-import java.nio.file.Files;
import static org.junit.Assert.assertEquals;
public class TestCleanerWithReplication extends CompactorTest {
private Path cmRootDirectory;
- private static FileSystem fs;
private static MiniDFSCluster miniDFSCluster;
private final String dbName = "TestCleanerWithReplication";
@Before
public void setup() throws Exception {
- conf = new HiveConf();
- TestTxnDbUtil.setConfValues(conf);
- TestTxnDbUtil.cleanDb(conf);
- conf.set("fs.defaultFS", fs.getUri().toString());
+ HiveConf conf = new HiveConf();
+ conf.set("fs.defaultFS", miniDFSCluster.getFileSystem().getUri().toString());
conf.setBoolVar(HiveConf.ConfVars.REPLCMENABLED, true);
- MetastoreConf.setBoolVar(conf, MetastoreConf.ConfVars.COMPACTOR_INITIATOR_ON, true);
- TestTxnDbUtil.prepDb(conf);
- ms = new HiveMetaStoreClient(conf);
- txnHandler = TxnUtils.getTxnStore(conf);
+ super.setup(conf);
Review Comment:
I removed `super` and changed the method to final to avoid unpleasant surprises and accidental changes in the future
https://github.com/apache/hive/pull/3514/commits/9be4f0e34aa3612fefd57b76fc7532d107e4fb33
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #3514: HIVE-26404: HMS memory leak when compaction cleaner fails to remove obsolete files
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3514:
URL: https://github.com/apache/hive/pull/3514#issuecomment-1252287854
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3514)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=BUG) [2 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3514&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3514&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3514&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3514&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3514&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak closed pull request #3514: HIVE-26404: HMS memory leak when compaction cleaner fails to remove obsolete files
Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #3514: HIVE-26404: HMS memory leak when compaction cleaner fails to remove obsolete files
URL: https://github.com/apache/hive/pull/3514
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a diff in pull request #3514: HIVE-26404: HMS memory leak when compaction cleaner fails to remove obsolete files
Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3514:
URL: https://github.com/apache/hive/pull/3514#discussion_r975251365
##########
ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/CompactorTest.java:
##########
@@ -181,7 +184,7 @@ protected Table newTable(String dbName, String tableName, boolean partitioned,
table.setTableName(tableName);
table.setDbName(dbName);
table.setOwner("me");
- table.setSd(newStorageDescriptor(getLocation(tableName, null), sortCols));
+ table.setSd(newStorageDescriptor(getLocation(tableName, null).toString(), sortCols));
Review Comment:
Restored the String return type https://github.com/apache/hive/pull/3514/commits/bff0fe3d4b941b01b89bc4d853d93841116c8bfd
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #3514: HIVE-26404: HMS memory leak when compaction cleaner fails to remove obsolete files
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3514:
URL: https://github.com/apache/hive/pull/3514#discussion_r974126899
##########
ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/CompactorTest.java:
##########
@@ -181,7 +184,7 @@ protected Table newTable(String dbName, String tableName, boolean partitioned,
table.setTableName(tableName);
table.setDbName(dbName);
table.setOwner("me");
- table.setSd(newStorageDescriptor(getLocation(tableName, null), sortCols));
+ table.setSd(newStorageDescriptor(getLocation(tableName, null).toString(), sortCols));
Review Comment:
minor: getLocation returning String was more natural here
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #3514: HIVE-26404: HMS memory leak when compaction cleaner fails to remove obsolete files
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3514:
URL: https://github.com/apache/hive/pull/3514#issuecomment-1249391895
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3514)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3514&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3514&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3514&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3514&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3514&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #3514: HIVE-26404: HMS memory leak when compaction cleaner fails to remove obsolete files
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3514:
URL: https://github.com/apache/hive/pull/3514#discussion_r974131463
##########
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCleanerWithReplication.java:
##########
@@ -44,34 +39,25 @@
import org.junit.Test;
import javax.security.auth.login.LoginException;
-import java.io.File;
import java.io.IOException;
-import java.nio.file.Files;
import static org.junit.Assert.assertEquals;
public class TestCleanerWithReplication extends CompactorTest {
private Path cmRootDirectory;
- private static FileSystem fs;
private static MiniDFSCluster miniDFSCluster;
private final String dbName = "TestCleanerWithReplication";
@Before
public void setup() throws Exception {
- conf = new HiveConf();
- TestTxnDbUtil.setConfValues(conf);
- TestTxnDbUtil.cleanDb(conf);
- conf.set("fs.defaultFS", fs.getUri().toString());
+ HiveConf conf = new HiveConf();
+ conf.set("fs.defaultFS", miniDFSCluster.getFileSystem().getUri().toString());
conf.setBoolVar(HiveConf.ConfVars.REPLCMENABLED, true);
- MetastoreConf.setBoolVar(conf, MetastoreConf.ConfVars.COMPACTOR_INITIATOR_ON, true);
- TestTxnDbUtil.prepDb(conf);
- ms = new HiveMetaStoreClient(conf);
- txnHandler = TxnUtils.getTxnStore(conf);
+ super.setup(conf);
Review Comment:
minor: no need for super keyword here, setup(conf) is only defined in parent class
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #3514: HIVE-26404: HMS memory leak when compaction cleaner fails to remove obsolete files
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3514:
URL: https://github.com/apache/hive/pull/3514#issuecomment-1248685147
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3514)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3514&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3514&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3514&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3514&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3514&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3514&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org