[
https://issues.apache.org/jira/browse/HDFS-6757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14077857#comment-14077857
]
Daryn Sharp commented on HDFS-6757:
-----------------------------------
I like {{LeaseManager}} tracking inodes rather than paths! It's something
Kihwal and I have long been meaning to do. I have two primary areas of
concern. First, we need to ensure that error conditions from bugs are not
silently tolerated. I've cited them in the following list. Second, that we
haven't created an incompatibility, discusses at the end.
# Why not track inodes directly instead of via Longs? It will avoid
unnecessary lookups in multiple places.
# Minor, {{FSEditLogLoader#applyEditLogOp}} has a line that overflows 80-chars.
# {{FSEditLogLoader#applyEditLogOp}} for a close op constructs a list so it can
call {{LeaseManager#removeLeases}}. That skips non-existent leases because
it's also invoked by {{FSNamesystem#removePathAndBlocks}} during deletes and
not all removed inodes may have a lease. 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}}?
# 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?
# In {{FSNamesystem#getCompleteBlocksTotal}}, please add spaces after ifs and
curlies around the 1-statement bodies. I know it's just unindented existing
code but it should be consistent with the coding standards.
# {{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?
# {{LeaseManager#getPaths}} used to return {{Collection}} but when renamed to
{{getFiles}} it returns {{HashSet}}. It would be better to either retain
Collection or use generic Set for the signature.
A possible compatibility issue due to a subtle functionality change: Renames
no longer reassign a lease which I think makes sense going forward since we
aren't tracking paths anymore. However, will backing up to a prior hdfs
version (with the same image format) have issues? I haven't traced the code
due to time constraints. 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?
> 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)
