Details
-
Type:
Sub-task
-
Status: Resolved
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: Heterogeneous Storage (HDFS-2832)
-
Fix Version/s: Heterogeneous Storage (HDFS-2832)
-
Component/s: datanode
-
Labels:None
-
Hadoop Flags:Reviewed
Description
The perVolumeReplicaMap in FsDatasetImpl.java is not necessary and can be removed. We continue to use the existing volumeMap.
Activity
Updated patch fixes an unrelated bug exposed by the earlier patch. DatanodeStorage was not overriding Object.equals() and Object.hashCode().
Show
Arpit Agarwal
added a comment - Updated patch fixes an unrelated bug exposed by the earlier patch. DatanodeStorage was not overriding Object.equals() and Object.hashCode() .
+1 patch looks good.
A minor comment: the type of volumes.volumes is already List<FsVolumeImpl>. The for loop below should be "for(FsVolumeImpl v : : volumes.volumes)". Then the cast in "((FsVolumeImpl) v).toDatanodeStorage()" is unnecessary.
+ for (FsVolumeSpi v : volumes.volumes) { + ArrayList<ReplicaInfo> finalizedList = finalized.get(v.getStorageID()); + ArrayList<ReplicaInfo> ucList = uc.get(v.getStorageID()); + blockReportsMap.put(((FsVolumeImpl) v).toDatanodeStorage(), + new BlockListAsLongs(finalizedList, ucList)); + }
Show
Tsz Wo Nicholas Sze
added a comment - +1 patch looks good.
A minor comment: the type of volumes.volumes is already List<FsVolumeImpl>. The for loop below should be "for(FsVolumeImpl v : : volumes.volumes)". Then the cast in "((FsVolumeImpl) v).toDatanodeStorage()" is unnecessary.
+ for (FsVolumeSpi v : volumes.volumes) {
+ ArrayList<ReplicaInfo> finalizedList = finalized.get(v.getStorageID());
+ ArrayList<ReplicaInfo> ucList = uc.get(v.getStorageID());
+ blockReportsMap.put(((FsVolumeImpl) v).toDatanodeStorage(),
+ new BlockListAsLongs(finalizedList, ucList));
+ }
Thanks for the quick review Nicholas! Updated patch with your feedback attached for reference.
I will commit this shortly.
Show
Arpit Agarwal
added a comment - Thanks for the quick review Nicholas! Updated patch with your feedback attached for reference.
I will commit this shortly.
I committed this, thanks again Nicholas.
Show
Arpit Agarwal
added a comment - I committed this, thanks again Nicholas.
Updated patch to follow - this one breaks a few tests.