[
https://issues.apache.org/jira/browse/HDFS-11100?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15765522#comment-15765522
]
Wei-Chiu Chuang edited comment on HDFS-11100 at 12/21/16 5:31 AM:
------------------------------------------------------------------
Thanks [~jzhuge] for the code contribution! I've completed my first review of
the patch.
My review notes are as follows:
# The patch adds an extra for loop and we should be careful to avoid pausing
NameNode from other operations for too long.
# Given that the original method body is essentially a -breadth-first-search-
(edit: my bad. This is a DFS), it should be easy to separate from the new code,
could we refactor the new code into a new method, to make it easier to
understand?
# One nit: the following line is repeated for every child inodes which only
need to be done once. Performance-wise this shouldn't have much impact.
{code}
// checkStickyBit only uses 2 entries in childInodeAttrs
childInodeAttrs[parentIdx] = inodeAttr;
{code}
# In the patch, an INodeAttributes array is instantiated per INodeDirectory.
This can be a performance hitter. Would it make sense to optimize it? For
example, pre-initialize this array, and double its length if
{{childComponents.length}} is larger than {{childInodeAttrs.length}}. We can
file a new jira for the optimization if you think this is reasonable.
was (Author: jojochuang):
Thanks [~jzhuge] for the code contribution! I've completed my first review of
the patch.
My review notes are as follows:
# The patch adds an extra for loop and we should be careful to avoid pausing
NameNode from other operations for too long.
# Given that the original method body is essentially a breadth-first-search, it
should be easy to separate from the new code, could we refactor the new code
into a new method, to make it easier to understand?
# One nit: the following line is repeated for every child inodes which only
need to be done once. Performance-wise this shouldn't have much impact.
{code}
// checkStickyBit only uses 2 entries in childInodeAttrs
childInodeAttrs[parentIdx] = inodeAttr;
{code}
# In the patch, an INodeAttributes array is instantiated per INodeDirectory.
This can be a performance hitter. Would it make sense to optimize it? For
example, pre-initialize this array, and double its length if
{{childComponents.length}} is larger than {{childInodeAttrs.length}}. We can
file a new jira for the optimization if you think this is reasonable.
> Recursively deleting file protected by sticky bit should fail
> -------------------------------------------------------------
>
> Key: HDFS-11100
> URL: https://issues.apache.org/jira/browse/HDFS-11100
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: fs
> Affects Versions: 2.6.0
> Reporter: John Zhuge
> Assignee: John Zhuge
> Priority: Critical
> Labels: permissions
> Attachments: HDFS-11100.001.patch, HDFS-11100.002.patch,
> HDFS-11100.003.patch, hdfs_cmds
>
>
> Recursively deleting a directory that contains files or directories protected
> by sticky bit should fail but it doesn't in HDFS. In the case below,
> {{/tmp/test/sticky_dir/f2}} is protected by sticky bit, thus recursive
> deleting {{/tmp/test/sticky_dir}} should fail.
> {noformat}
> + hdfs dfs -ls -R /tmp/test
> drwxrwxrwt - jzhuge supergroup 0 2016-11-03 18:08
> /tmp/test/sticky_dir
> -rwxrwxrwx 1 jzhuge supergroup 0 2016-11-03 18:08
> /tmp/test/sticky_dir/f2
> + sudo -u hadoop hdfs dfs -rm -skipTrash /tmp/test/sticky_dir/f2
> rm: Permission denied by sticky bit: user=hadoop,
> path="/tmp/test/sticky_dir/f2":jzhuge:supergroup:-rwxrwxrwx,
> parent="/tmp/test/sticky_dir":jzhuge:supergroup:drwxrwxrwt
> + sudo -u hadoop hdfs dfs -rm -r -skipTrash /tmp/test/sticky_dir
> Deleted /tmp/test/sticky_dir
> {noformat}
> Centos 6.4 behavior:
> {noformat}
> $ ls -lR /tmp/test
> /tmp/test:
> total 4
> drwxrwxrwt 2 systest systest 4096 Nov 3 18:36 sbit
> /tmp/test/sbit:
> total 0
> -rw-rw-rw- 1 systest systest 0 Nov 2 13:45 f2
> $ sudo -u mapred rm -fr /tmp/test/sbit
> rm: cannot remove `/tmp/test/sbit/f2': Operation not permitted
> $ chmod -t /tmp/test/sbit
> $ sudo -u mapred rm -fr /tmp/test/sbit
> {noformat}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
