[
https://issues.apache.org/jira/browse/HDFS-8266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14568815#comment-14568815
]
GAO Rui commented on HDFS-8266:
-------------------------------
Hi [~rakeshr], I have some advice for the patch [^HDFS-8266-HDFS-7285-01.patch]:
(1).code style consistency.
{code:java}
// create erasure zone
fs.createErasureCodingZone(zone, null, 0);
DFSTestUtil.createFile(fs, zoneFile, len, (short) 1, 0xFEED);
String contents = DFSTestUtil.readFile(fs, zoneFile);
final Path snap1 = fs.createSnapshot(zoneParent, "snap1");
final Path snap1Zone = new Path(snap1, zone.getName());
assertEquals("Got unexpected erasure zone path", zone.toString(), fs
.getErasureCodingZone(snap1Zone).getDir().toString());
{code}
and
{code:java}
// Create the erasure zone again
fs.createErasureCodingZone(zone, null, 0);
final Path snap3 = fs.createSnapshot(zoneParent, "snap3");
final Path snap3Zone = new Path(snap3, zone.getName());
// Check that snap3's EZ has the correct settings
ErasureCodingZone ezSnap3 = fs.getErasureCodingZone(snap3Zone);
assertEquals("Got unexpected erasure zone path", zone.toString(), ezSnap3
.getDir().toString());
{code}
shared the same actions but were written in different style. Maybe we can
change all the similar codes written in the former style to the latter. like:
{code:java}
// create erasure zone
fs.createErasureCodingZone(zone, null, 0);
DFSTestUtil.createFile(fs, zoneFile, len, (short) 1, 0xFEED);
String contents = DFSTestUtil.readFile(fs, zoneFile);
final Path snap1 = fs.createSnapshot(zoneParent, "snap1");
final Path snap1Zone = new Path(snap1, zone.getName());
ErasureCodingZone ezSnap1 = fs.getErasureCodingZone(snap1Zone);
assertEquals("Got unexpected erasure zone path", zone.toString(), ezSnap1
.getDir().toString());
{code}
It might make the future reading and revising of the code easier.
(2).
In code like bellow:
{code:java}
fs.getErasureCodingZone(snap1Zone).getDir().toString()
{code}
the return type of {{fs.getErasureCodingZone(snap1Zone).getDir()}} is already
{{String}}, so I think we don't need to call {{.toString()}}.
(3).
{code:java}
// Check that older snapshots still have the old EZ settings
ErasureCodingZone ezSnap1 = fs.getErasureCodingZone(snap1Zone);
assertEquals("Got unexpected erasure zone path", zone.toString(), ezSnap1
.getDir().toString());
{code}
I think we could not confirm the older snapshots still have the old EZ settings
here, cause {{snap1Zone}} has the exactly same value as {{snap3Zone}}. Maybe,
we can make different settings of old EZ and new EZ, so that we can confirm the
older snapshot keep the old EZ settings.
(4).
In method testSnapshotsOnECZoneDir(), ECZone was created based on {{zone(value:
"/zone")}}. When we call {{fs.getErasureCodingZone()}} respectively with
parameter
{{snap1Zone(value:"/zone/.snapshot/snap1/zone")}}and{{snap1(value:"/zone/.snapshot/snap1")}},
I think {{fs.getErasureCodingZone(snap1)}} actually returns the ECZone set to
{{zone(value:"/zone"), and {{fs.getErasureCodingZone(snap1Zone)}} returns the
ECZone set in the process of snapshot, right? If it's right, then ECZone can
be nested, so does {{fs.getErasureCodingZone()}} return the nearest parent dir
which is ECZone of the Path parameter (or the Path parameter itself if the Path
parameter is ECZone)?
Actually, this jira is contained by the umbrella jira [HDFS-8197], we created
this jira for EC system test, not unit test. But, I think that's great to
finish both the unit test and the system test related to snapshot with EC
files. When it comes to system test, we have the image of testing snapshot
functionality by using it like an end user(of cause can be imitated by testing
scripts).
> Erasure Coding: Test of snapshot with EC files
> ----------------------------------------------
>
> Key: HDFS-8266
> URL: https://issues.apache.org/jira/browse/HDFS-8266
> Project: Hadoop HDFS
> Issue Type: Test
> Affects Versions: HDFS-7285
> Reporter: GAO Rui
> Attachments: HDFS-8266-HDFS-7285-00.patch,
> HDFS-8266-HDFS-7285-01.patch, HDFS-8266-HDFS-7285-01.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
