[
https://issues.apache.org/jira/browse/HDFS-12217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16108147#comment-16108147
]
Wei-Chiu Chuang commented on HDFS-12217:
----------------------------------------
Thanks [~manojg] [~yzhangal].
One nit I found is that {{LeaseManager#getINodeWithLeases()}} seems to be a
test-only method. If so, it doesn't need public modifier (package private is
sufficient) and we can also add a {{@VisibleForTesting }} annotation.
On a separate note, and this is totally unrelated to this patch. It looks like
{{LeaseManager#addLease}} assumes the inodeId is an id for an INodeFile, which
makes perfect sense. The member variable {{leasesById}} also implicitly makes
the assumption that the id is for an INodeFile. However, there's no assertion
to ensure it is the case in the future. I wonder if it makes sense to add a new
{{addLease(String, INodeFile)}} method to make sure only INodeFile is passed
in. So it would go like:
{code}
Lease addLease(String holder, INodeFile inodeFile) {
addLease(holder, inodeFile.getId());
}
private synchronized Lease addLease(String holder, long inodeId) {
...
}
{code}
Note that in your patch, {{DirectorySnapshottableFeature#addSnapshot}} would
capture exception, but doesn't log the exception. I also think that in addition
to the snapshot name, you should print the snapshot root path for the error
message.
> HDFS snapshots doesn't capture all open files when one of the open files is
> deleted
> -----------------------------------------------------------------------------------
>
> Key: HDFS-12217
> URL: https://issues.apache.org/jira/browse/HDFS-12217
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: snapshots
> Affects Versions: 3.0.0-alpha1
> Reporter: Manoj Govindassamy
> Assignee: Manoj Govindassamy
> Attachments: HDFS-12217.01.patch, HDFS-12217.02.patch,
> HDFS-12217.03.patch
>
>
> With the fix for HDFS-11402, HDFS Snapshots can additionally capture all the
> open files. Just like all other files, these open files in the snapshots will
> remain immutable. But, sometimes it is found that snapshots fail to capture
> all the open files in the system.
> Under the following conditions, LeaseManager will fail to find INode
> corresponding to an active lease
> * a file is opened for writing (LeaseManager allots a lease), and
> * the same file is deleted while it is still open for writing and having
> active lease, and
> * the same file is not referenced in any other Snapshots/Trash
> {{INode[] LeaseManager#getINodesWithLease()}} can thus return null for few
> leases there by causing the caller to trip over and not return all the open
> files needed by the snapshot manager.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
