[
https://issues.apache.org/jira/browse/HDFS-13118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16356962#comment-16356962
]
Shashikant Banerjee commented on HDFS-13118:
--------------------------------------------
Thanks [~ehiggs], for working on this. I had a quick look at this and patch
looks good to me overall. Some minor comments:
1.nit: SnapshotDiffListingInfo..java:90 : extra space.
2.SnapshotDiffListingInfo..java: 107
{code:java}
createdList.add(new DiffReportListingEntry(DIRECTORY, dirId,
created.getId(), path, created.isReference(), null));{code}
I am just curious to know why the INodeType fileld being harcoded to
"DIRECTORY" here?
In the created list for directory diff, we can have files/directory/symlinks...
I think we should check what exactly the inode being added in the created list
(directory/file/symlink).
3.SnapshotDiffListingInfo..java: 129:
{code:java}
final DiffReportListingEntry e = target != null ?
new DiffReportListingEntry(DIRECTORY, dirId, d.getId(), path,
true, target) :
new DiffReportListingEntry(DIRECTORY, dirId, d.getId(), path,
false, null);
deletedList.add(e);{code}
While adding in the deleted list also, should we not check what the actual
inode is instead of hardcoding it as "DIRECTORY"?
4.SnapshotDiffReportGenerator:237
{code:java}
private List<DiffReportEntry> generateReport(
DiffReportListingEntry modified) {
List<DiffReportEntry> diffReportList = new ChunkedArrayList<>();
ChildrenDiff list = dirDiffMap.get(modified.getDirId());
for (DiffReportListingEntry created : list.getCreatedList()) {
RenameEntry entry = renameMap.get(created.getFileId());
if (entry == null || !entry.isRename()) {
diffReportList.add(new DiffReportEntry(
modified.getINodeType().toSnapshotDiffReportINodeType(),
isFromEarlier ? DiffType.CREATE : DiffType.DELETE,
created.getSourcePath()));
}
}
for (DiffReportListingEntry deleted : list.getDeletedList()) {
RenameEntry entry = renameMap.get(deleted.getFileId());
if (entry != null && entry.isRename()) {
diffReportList.add(new DiffReportEntry(
modified.getINodeType().toSnapshotDiffReportINodeType(),
DiffType.RENAME,
isFromEarlier ? entry.getSourcePath() : entry.getTargetPath(),
isFromEarlier ? entry.getTargetPath() : entry.getSourcePath()));
} else {
diffReportList.add(new DiffReportEntry(
modified.getINodeType().toSnapshotDiffReportINodeType(),
isFromEarlier ? DiffType.DELETE : DiffType.CREATE,
deleted.getSourcePath()));
}
}
return diffReportList;
}
{code}
For each modified directory , we get the childrenList here and determine
whether its a Rename, create or delete op. But seems like for every
create/delete/rename entry here we putting INodeType as the
"modified.getINodeType()" while adding it to the diffReportList which i think
should be created.getINodeType(), entry.getINodeType() , deleted.getINodeType()
respectively.
> SnapshotDiffReport should provide the INode type
> ------------------------------------------------
>
> Key: HDFS-13118
> URL: https://issues.apache.org/jira/browse/HDFS-13118
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: snapshots
> Affects Versions: 3.0.0
> Reporter: Ewan Higgs
> Assignee: Ewan Higgs
> Priority: Major
> Attachments: HDFS-13118.001.patch
>
>
> Currently the snapshot diff report will list which inodes were added,
> removed, renamed, etc. But to see what the INode actually is, we need to
> actually access the underlying snapshot - and this is cumbersome to do
> programmatically when the snapshot diff already has the information.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
