Skip to content

HIVE-28820: HMS might only creates the external directory and not the managed directory #6717

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.

Sign up for GitHub

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

Open
DanielZhu58 wants to merge 1 commit into
base: master
Choose a base branch
from
+64 −49
Open

HIVE-28820: HMS might only creates the external directory and not the managed directory #6717

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 64 additions & 49 deletions ...-server/src/main/java/org/apache/hadoop/hive/metastore/handler/CreateDatabaseHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ protected CreateDatabaseResult execute() throws TException, IOException {
Map<String, String> transactionalListenersResponses = Collections.emptyMap();
Path dbExtPath = new Path(db.getLocationUri());
Path dbMgdPath = db.getManagedLocationUri() != null ? new Path(db.getManagedLocationUri()) : null;

// HIVE-28820: create the default managed database directory even when MANAGEDLOCATION
// is not explicitly specified. Do not persist this default path into Database.managedLocationUri.
Path managedPathToCreate = dbMgdPath != null ? dbMgdPath : wh.getDefaultDatabasePath(db.getName(), false);
boolean explicitManagedLocation = dbMgdPath != null;
boolean isInTest = MetastoreConf.getBoolVar(handler.getConf(), HIVE_IN_TEST);
try {
Database authDb = new Database(db);
Expand All @@ -97,53 +102,20 @@ protected CreateDatabaseResult execute() throws TException, IOException {
madeExternalDir = true;
}
} else {
if (dbMgdPath != null) {
try {
// Since this may be done as random user (if doAs=true) he may not have access
// to the managed directory. We run this as an admin user
madeManagedDir = UserGroupInformation.getLoginUser().doAs((PrivilegedExceptionAction<Boolean>) () -> {
if (!wh.isDir(dbMgdPath)) {
LOG.info("Creating database path in managed directory {}", dbMgdPath);
if (!wh.mkdirs(dbMgdPath)) {
throw new MetaException("Unable to create database managed path " + dbMgdPath +
", failed to create database " + db.getName());
}
return true;
}
return false;
});
if (madeManagedDir) {
LOG.info("Created database path in managed directory {}", dbMgdPath);
} else if (!isInTest || !isDbReplicationTarget(db)) { // Hive replication tests doesn't drop the db after each test
throw new MetaException("Unable to create database managed directory " + dbMgdPath +
", failed to create database " + db.getName());
}
} catch (IOException | InterruptedException e) {
throw new MetaException(
"Unable to create database managed directory " + dbMgdPath + ", failed to create database " +
db.getName() + ":" + e.getMessage());
}
madeManagedDir = createDbDirectory(managedPathToCreate, true, "managed", true);
if (madeManagedDir) {
LOG.info("Created database path in managed directory {}", managedPathToCreate);
} else if (explicitManagedLocation && (!isInTest || !isDbReplicationTarget(db))) {
throw new MetaException("Unable to create database managed directory " + managedPathToCreate +
", failed to create database " + db.getName());
}
try {
madeExternalDir = UserGroupInformation.getCurrentUser().doAs((PrivilegedExceptionAction<Boolean>) () -> {
if (!wh.isDir(dbExtPath)) {
LOG.info("Creating database path in external directory {}", dbExtPath);
return wh.mkdirs(dbExtPath);
}
return false;
});
if (madeExternalDir) {
LOG.info("Created database path in external directory {}", dbExtPath);
} else {
LOG.warn(
"Failed to create external path {} for database {}. " +
"This may result in access not being allowed if the StorageBasedAuthorizationProvider is enabled",
dbExtPath, db.getName());
}
} catch (IOException | InterruptedException | UndeclaredThrowableException e) {
throw new MetaException("Failed to create external path " + dbExtPath + " for database " + db.getName() +
". This may result in access not being allowed if the " +
"StorageBasedAuthorizationProvider is enabled: " + e.getMessage());
madeExternalDir = createDbDirectory(dbExtPath, false, "external", false);
if (madeExternalDir) {
LOG.info("Created database path in external directory {}", dbExtPath);
} else {
LOG.warn("Failed to create external path {} for database {}. " +
"This may result in access not being allowed if the StorageBasedAuthorizationProvider is enabled",
dbExtPath, db.getName());
}
}

Expand All @@ -166,15 +138,15 @@ protected CreateDatabaseResult execute() throws TException, IOException {
wh.deleteDir(dbMgdPath, true, db);
}
} else {
if (madeManagedDir && dbMgdPath != null) {
if (madeManagedDir) {
try {
UserGroupInformation.getLoginUser().doAs((PrivilegedExceptionAction<Void>) () -> {
wh.deleteDir(dbMgdPath, true, db);
wh.deleteDir(managedPathToCreate, true, db);
return null;
});
} catch (IOException | InterruptedException e) {
LOG.error("Couldn't delete managed directory {} after it was created for database {} {}",
dbMgdPath, db.getName(), e.getMessage());
managedPathToCreate, db.getName(), e.getMessage());
}
}

Expand Down Expand Up @@ -241,6 +213,9 @@ protected void beforeExecute() throws TException, IOException {
db.setManagedLocationUri(dbMgdPath.toString());
}

db.setLocationUri(dbExtPath.toString());
db.setManagedLocationUri(dbMgdPath.toString());

if (db.getOwnerName() == null){
try {
db.setOwnerName(SecurityUtils.getUGI().getShortUserName());
Expand Down Expand Up @@ -269,4 +244,44 @@ public record CreateDatabaseResult(boolean success,
Map<String, String> transactionalListenersResponses) implements Result {

}

/**
* Creates the given database directory (managed or external) as the given user,
* running the actual mkdir as an admin (login) or current user depending on runAsLoginUser.
*
* @param path the directory path to create
* @param runAsLoginUser true to run as the login (admin) user (used for managed dir,
* since the calling user may not have access to it),
* false to run as the current user (used for external dir)
* @param dirLabel a short label ("managed"/"external") used only for log/error messages
* @param throwOnMkdirFailure true to throw the exception about create database dir
* @return true if the directory was created by this call, false if it already existed
* @throws MetaException if directory creation fails
*/
private boolean createDbDirectory(Path path, boolean runAsLoginUser, String dirLabel,
boolean throwOnMkdirFailure) throws MetaException {
try {
UserGroupInformation ugi = runAsLoginUser
? UserGroupInformation.getLoginUser()
: UserGroupInformation.getCurrentUser();
return ugi.doAs((PrivilegedExceptionAction<Boolean>) () -> {
if (!wh.isDir(path)) {
LOG.info("Creating database path in {} directory {}", dirLabel, path);
if (!wh.mkdirs(path)) {
if (throwOnMkdirFailure) {
throw new MetaException("Unable to create database " + dirLabel + " path " + path +
", failed to create database " + db.getName());
}
return false;
}
return true;
}
return false;
});
} catch (IOException | InterruptedException | UndeclaredThrowableException e) {
throw new MetaException("Unable to create database " + dirLabel + " directory " + path +
", failed to create database " + db.getName() + ": " + e.getMessage());
}
}

}
Loading