Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow ignoring schema location cleanup #10067

Conversation

findepi
Copy link
Member

@findepi findepi commented Nov 25, 2021

It is possible that schema location is set to something invalid, or
something that used to be valid but is not anymore. In such case, schema
location cleanup would fail.

It is possible that schema location is set to something invalid, or
something that used to be valid but is not anymore. In such case, schema
location cleanup would fail.
@findepi findepi merged commit 8274da8 into trinodb:master Nov 25, 2021
@findepi findepi deleted the findepi/allow-ignoring-schema-location-cleanup-634ea3 branch November 25, 2021 13:31
@github-actions github-actions bot added this to the 365 milestone Nov 25, 2021
@findepi
Copy link
Member Author

findepi commented Nov 26, 2021

@losipiuk i just found a related piece of code in thrift metastore handling of DROP TABLE.
it's opt-in, but then it suppresses exceptions

private static void deleteDirRecursive(HdfsContext context, HdfsEnvironment hdfsEnvironment, Path path)
{
try {
hdfsEnvironment.getFileSystem(context, path).delete(path, true);
}
catch (IOException | RuntimeException e) {
// don't fail if unable to delete path
log.warn(e, "Failed to delete path: " + path.toString());

thoughts?

@losipiuk
Copy link
Member

@losipiuk i just found a related piece of code in thrift metastore handling of DROP TABLE. it's opt-in, but then it suppresses exceptions

private static void deleteDirRecursive(HdfsContext context, HdfsEnvironment hdfsEnvironment, Path path)
{
try {
hdfsEnvironment.getFileSystem(context, path).delete(path, true);
}
catch (IOException | RuntimeException e) {
// don't fail if unable to delete path
log.warn(e, "Failed to delete path: " + path.toString());

thoughts?

It is more natural to me to ignore exceptions as an opt-in. Not the other way around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants