[
https://issues.apache.org/jira/browse/HDFS-6757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14079892#comment-14079892
]
Haohui Mai commented on HDFS-6757:
----------------------------------
Thanks [~daryn] for the review. I'm updating my patch to address the comments.
Here are some of my thoughts:
bq. Why not track inodes directly instead of via Longs? It will avoid
unnecessary lookups in multiple places.
It might not be sufficient as previous implementation of snapshot might replace
an inode with its subclass (e.g., replace {{INodeDirectory}} to
{{INodeDirectoryWithSnapshots}}). I'm not sure whether the behavior still
exists today, I think [~jingzhao] might have a better idea on whether this is
okay. [~jingzhao], can you comment on this?
{quote}
However, isn't a bug if there's no lease during a close op? Perhaps
removeLeases skips missing leases and calls a removeLease that requires a
lease, and the close edit op calls removeLease?
FSImageFormatPBINode#serializeFileUCSection skips non-existent inodes reported
from the lease manager. When would this happen other than due to bug that
corrupted the lease manager?
{quote}
I think that your points are quite fair. My thinking is that even though the
lease manager is buggy, NN should continue to function when replay edit logs /
saving namespaces instead of crashing. Though not ideal, but it allows NN to
tolerate the bugs and to treat them as lease recoveries. Please see
{{TestSaveNamespace#testSaveNamespaceWithDanglingLease}} for more details.
I'm updating my patch to print out warnings to help developers to catch these
bugs.
bq. In serialization's special handling of UC snapshot files: Is this for all
files in the mutable snapshotted directory, or immutable files only in a
snapshot? If the latter, isn't it a bug that the lease wasn't already revoked?
I think that the use case is that the user takes a snapshot which contains an
opened file, and then deletes the file subsequently. Maybe [~jingzhao] will
have a better idea on this.
bq. My concern is how the current NN will react to a dangling lease on the
origin of the renamed path. What happens when it saves the image, checkpoints,
applies edits for subsequent creates of the origin path, when another client
tries to recreate the origin path, etc?
You're right that the new lease manager won't (and doesn't need to) update the
lease during the renames, but I'm not sure that I fully understand your
comments. Are you concerned about the new lease manager will mix the leases
between the original and the renamed paths? Based on my understanding it should
not happen because inode ids are unique, but I might misunderstand your
comments so please feel free to provide more details.
> Simplify lease manager with INodeID
> -----------------------------------
>
> Key: HDFS-6757
> URL: https://issues.apache.org/jira/browse/HDFS-6757
> Project: Hadoop HDFS
> Issue Type: Improvement
> Reporter: Haohui Mai
> Assignee: Haohui Mai
> Attachments: HDFS-6757.000.patch, HDFS-6757.001.patch,
> HDFS-6757.002.patch, HDFS-6757.003.patch
>
>
> Currently the lease manager records leases based on path instead of inode
> ids. Therefore, the lease manager needs to carefully keep track of the path
> of active leases during renames and deletes. This can be a non-trivial task.
> This jira proposes to simplify the logic by tracking leases using inodeids
> instead of paths.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
