HIVE-29106: When drop a table, there is no need to verify the directo… #5997
Conversation
|
@deniskuzZ Please review the code.Thanks |
asf-ci-hive
added
tests pending
tests failed
and removed
tests failed
tests pending
labels
|
@ayushtkn Please help review the code.Thank you. |
| @@ -3050,18 +3050,6 @@ private boolean drop_table_core(final RawStore ms, final String catName, final S | |||
| firePreEvent(new PreDropTableEvent(tbl, deleteData, this)); | |||
|
|
|||
| tableDataShouldBeDeleted = checkTableDataShouldBeDeleted(tbl, deleteData); | |||
| if (tableDataShouldBeDeleted && tbl.getSd().getLocation() != null) { | |||
There was a problem hiding this comment.
@zxl-333 initially, you added the permission check in HIVE-28804, mentioning that some Hive clusters don't use Ranger and now you're removing it?
I'm not sure I understand the reasoning.
I see you've added ms.rollbackTransaction(). So the flow is: make a DB call to drop the metadata, and then depending on whether the directory on the filesystem was successfully removed, you either commit or roll back, right?
There was a problem hiding this comment.
@deniskuzZ When Ranger is not enabled, there is no issue with HIVE-28804. However, when Ranger is enabled, the HMS code fails to retrieve authorization information from Ranger during verification, leading to the following situation: even though write permissions for the table have been granted through Ranger, if the HDFS ACL permissions do not include write access, the user will be prompted with a "no write permission" error, resulting in the failure to drop the table.
Now, we need to resolve the issue where table deletion fails when permissions are available and succeeds when permissions are unavailable, regardless of whether Ranger is enabled. The solution is to skip the verification of HDFS ACL permissions for database and table directories. Instead, the permission verification during deletion should be entrusted to the NameNode. If the deletion fails due to permission issues, the metadata must also be rolled back, and the engine side will receive the exception information about the deletion failure.
There was a problem hiding this comment.
@zxl-333, please share detailed step-by-step reproduce. I don't recall any customer using Ranger raising escalations related to that functionality.
There was a problem hiding this comment.
There was a problem hiding this comment.
@zxl-333 - Based on the scenario you mentioned, I believe this is a Hive or Ranger config issue.
Regarding Hive config, what is hive.server2.enable.doAs set to? It should be set to true. By doing so, assuming that you are running queries from jdbc client like beeline, when an end-user runs some query, user 'hive' does all the operations in the HDFS. Now if an end-user drops a table, user 'hive' will try to drop the data for the table path and its partitions. Now user 'hive' is having issues with the permissions with the path that leads to the Ranger config issue.
Regarding ranger config, user 'hive' should be added in hdfs service in the ranger policy for hive warehouse table path, and I believe the issue here because of this missing policy in hdfs service. Also, when ranger policies are missing, the Hadoop ACLs permissions takes into effect.
So I feel, this is not a product issue, and you might have to abandon this patch.
There was a problem hiding this comment.
@saihemanth-cloudera @deniskuzZ
Do not use ranger. Refer to the steps in HIVE-18888.
When using ranger, even if ranger has been authorized, it may still result in the failure of the ACL write permission verification in HDFS, preventing the deletion of the table.
specific steps are as follows:
1、User_a has only read permissions for the test db and the following tables in HDFS.
hdfs://nn1/usr/warhouse/hive/test.db
2、However, now the read and write permissions for the 'test' db and its tables have been granted through the 'ranger' authorization.
3. If 'user_a' attempts to delete the 'test_1' table within the 'test' library, the following exception will be thrown
test_1 metadata not deleted since hdfs://nn//usr/warhouse/hive/test.db is not writable by user_a
The exception thrown above will result in the user being unable to delete even if the ranger authorization is granted.
Therefore, regardless of whether the ranger permission check is enabled or not, in order to ensure that users do not have read or write permissions for the database tables, the operation must fail and the metadata database transaction must be rolled back.
There was a problem hiding this comment.
From your comments, you seem to use Ranger only for metadata permissions and not file permissions (HDFS service is not being used in Ranger for auth OR there are no policies for user 'hive'). So you would rely on Hadoop ACLs for file authorization. So (2) you mentioned doesn't apply. And (3) is expected to happen and it should happen in the absence of Ranger related Hadoop policies.
There was a problem hiding this comment.
Yes. After enabling the ranger control of HDFS file permissions, it is usually not necessary to use HDFS's ACL permissions for re-authorization. If this modification is not made, then after using ranger to control HDFS permissions, it is still necessary to grant HDFS ACL permissions to the user. This is unacceptable because the user needs to be authorized twice for a table path.
There was a problem hiding this comment.
@zxl-333 -- Can you please give me a detailed repro of this issue (probably a doc with screenshots with ranger policies, core-site.xml (in Hadoop conf) etc.), as previously asked by @deniskuzZ as well.
Because I cannot get this whatsoever
then after using ranger to control HDFS permissions, it is still necessary to grant HDFS ACL permissions to the user.
If you are using to control HDFS permissions, then you don't need to grant the end user the required permissions.
I'm guessing that you are missing this config in your core-site.xml of Hadoop service:
<property> <name>hadoop.proxyuser.hive.hosts</name> <value>*</value> </property>
<property> <name>hadoop.proxyuser.hive.groups</name> <value>*</value> </property>
There was a problem hiding this comment.
The core-site has already set these parameters.
steps:
1、create database and table
2、Check the HDFS permissions for the database and table
3、Grant all permissions to the dmp_query user through the ranger authorization.
4、Verify whether the dmp_query user has write permissions
5、Use the dmp_query user to drop this table
We observed that although ranger was authorized, the inability to delete was caused by the permission verification stored in HDFS.
asf-ci-hive
added
tests pending
tests unstable
and removed
tests unstable
tests pending
labels
|
| } catch (Exception e) { | ||
| ms.rollbackTransaction(); | ||
| LOG.error("Failed to drop table " + name + ", rolling back transaction", e); | ||
| if (e instanceof MetaException) { |
There was a problem hiding this comment.
When the deletion of HDFS data fails, we ensure that the metadata data of HMS is rolled back.
There was a problem hiding this comment.
I meant, why do we need to remap the exception to MetaException?
| @@ -3216,7 +3217,8 @@ private void deleteDataExcludeCmroot(Path path, boolean ifPurge, boolean shouldE | |||
| wh.deleteDir(path, true, ifPurge, shouldEnableCm); | |||
| } | |||
| } catch (Exception e) { | |||
| LOG.error("Failed to delete directory: {}", path, e); | |||
| throw new MetaException("Failed to delete directory: " + path.toString() + " . Exception detail : " | |||
There was a problem hiding this comment.
keep the log.error and maybe throw IOException. that is not a MetaException
…ry permission
What changes were proposed in this pull request?
When drop a table, there is no need to verify the directory permission. An exception should be thrown when the table path deletion fails.
Why are the changes needed?
When HiveServer2 integrates Ranger for authentication, there will be a bug in the permission verification. Ranger has already granted write permissions for the database tables and even partitions. However, when attempting to delete a table, it still prompts that there is no permission for the database directory or the table directory, resulting in the inability to delete the table.
The correct solution is to remove the table without verifying the directory write permission, not to catch the exception when deleting the data directory, but to throw the exception instead, and at the same time roll back the metadata information.
Does this PR introduce any user-facing change?
No
How was this patch tested?