You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/12/15 13:40:05 UTC
[GitHub] [iceberg] lirui-apache opened a new issue #3750: Some tests are leaking temp folders
lirui-apache opened a new issue #3750:
URL: https://github.com/apache/iceberg/issues/3750
We encountered this issue with 0.11, but thought it might still apply to latest code.
Basically the issue is because test classes try to clean up temp folders by calling `java.io.File.delete`, which won't really delete the folder if it's not empty.
Problematic classes include:
`SparkTestBaseWithCatalog::dropWarehouse`
`TestHiveMetastore::stop`
`TestCatalogTableLoader::dropWarehouse`
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] lirui-apache commented on issue #3750: Some tests are leaking temp folders
Posted by GitBox <gi...@apache.org>.
lirui-apache commented on issue #3750:
URL: https://github.com/apache/iceberg/issues/3750#issuecomment-995829922
I tried deleting these folders with commons-io `FileUtils`, but then tests start to fail with metastore internal errors. I think it's because we didn't clean up some static fields in metastore between different tests.
Such as the thread local TxnStore in HMSHandler: https://github.com/apache/hive/blob/rel/release-2.3.8/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java#L249
and the connection pool in `TxnHandler`: https://github.com/apache/hive/blob/rel/release-2.3.8/metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java#L149
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on issue #3750: Some tests are leaking temp folders
Posted by GitBox <gi...@apache.org>.
pvary commented on issue #3750:
URL: https://github.com/apache/iceberg/issues/3750#issuecomment-995885394
@lirui-apache: If I recall correctly we usually start the HMS instance `BeforeClass` and close it in `AfterClass`. So deleting warehouse directories between tests would be counterproductive 😄
`TestHiveMetastore.reset()` tries to drop the extra databases, tables and folders. Is it called between tests?
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on issue #3750: Some tests are leaking temp folders
Posted by GitBox <gi...@apache.org>.
pvary commented on issue #3750:
URL: https://github.com/apache/iceberg/issues/3750#issuecomment-996537302
@lirui-apache: You are absolutely right that we should delete the warehouse folders in the `AfterClass`. I am surprised that the directory is not removed as we create it with `java.nio.file.Files.createTempDirectory("hive", asFileAttribute(fromString("rwxrwxrwx"))).toFile()`. From the doc:
> As with the createTempFile methods, this method is only part of a temporary-file facility. A shutdown-hook, or the File.deleteOnExit mechanism may be used to delete the directory automatically.
Maybe we just have to call after creation?
```
this.hiveLocalDir.deleteOnExit();
```
As for the `TestHiveMetastore .reset()`, I think it would be useful to call it in the `After` methods to separate the test cases.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary closed issue #3750: Some tests are leaking temp folders
Posted by GitBox <gi...@apache.org>.
pvary closed issue #3750:
URL: https://github.com/apache/iceberg/issues/3750
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] lirui-apache commented on issue #3750: Some tests are leaking temp folders
Posted by GitBox <gi...@apache.org>.
lirui-apache commented on issue #3750:
URL: https://github.com/apache/iceberg/issues/3750#issuecomment-996418064
Hi @pvary , I'm not trying to delete warehouse folders between test methods. I just want to make sure the clean up logic in `AfterClass` really does its job to delete these folders. Or do you mean even `AfterClass` shouldn't do this? Then I wonder where this can be done, because otherwise it slowly eats up the disk space in our CI runners.
I checked `TestHiveMetastore.reset()`, seems it's only called in the `After` method of some tests in `iceberg-mr`.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] lirui-apache commented on issue #3750: Some tests are leaking temp folders
Posted by GitBox <gi...@apache.org>.
lirui-apache commented on issue #3750:
URL: https://github.com/apache/iceberg/issues/3750#issuecomment-997862697
Thanks @pvary for confirming. Yeah a shutdown hook will do the trick, although we might still want to figure out why follow-up tests are relying on the previous temp folders not being deleted. I guess it indicates some more static fields we need to clear out.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org