[
https://issues.apache.org/jira/browse/HDFS-11912?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16033821#comment-16033821
]
Manoj Govindassamy edited comment on HDFS-11912 at 6/1/17 10:27 PM:
--------------------------------------------------------------------
Thanks for contributing this patch [~ghuangups]. Looks good overall. Few
comments from the quick look. Will add more comments later.
In HDFS-9406, snapshot operations were believed to causing metadata
inconsistencies in the fsimage. Can you please try running this new test
without the fix for HDFS-9406 and see if it can recreate the problem?
1.
{noformat}
if (randomNum > currentWeightSum && randomNum <= (currentWeightSum +
currentValue.getWeight())) {
snapshotRandomOp = currentValue;
break;
}
{noformat}
Shouldn't the check be just (randomNum < (currentWeightSum +
currentValue.getWeight())
2.
{noformat}
private static MiniDFSCluster cluster;
private static DistributedFileSystem hdfs;
private static Random GENERATOR = null;
{noformat}
Above class members need not be static.
3.
{{FileSystemOperations}} and {{SnapshotOperations}} are very similar except for
enum values and weights. Code duplication here can be avoided if we can merge
these two enums into one and expose proper methods.
4.
{noformat}
// Set
Random RANDOM = new Random();
long seed = RANDOM.nextLong();
GENERATOR = new Random(seed);
{noformat}
Any specific reason why a simple seed like System.currentTimeMillis() will not
be useful here ? Here seed is generated from random, which is inturn is not
seeded. Also, RANDOM need not be all caps.
5.
{noformat}
int fileLen = new Random().nextInt(MAX_NUM_FILE_LENGTH);
createFiles(testDirString, fileLen);
{noformat}
GENERATOR random can be used here instead of creating a new one.
6.
{noformat}
// Create files in a directory with random depth, ranging from 0-10.
for (int i = 0; i < TOTAL_BLOCKS; i += fileLength) {
{noformat}
Is this TOTAL_BLOCKS are total files ?
7.
{noformat}
private String GetNewPathString(String originalString,
{noformat}
Metnhod name should be in camel case, like getNewPathString()
was (Author: manojg):
Thanks for contributing this patch [~ghuangups]. Few comments from the quick
look. Will add more comments later.
In HDFS-9406, snapshot operations were believed to causing metadata
inconsistencies in the fsimage. Can you please try running this new test
without the fix for HDFS-9406 and see if it can recreate the problem?
1.
{noformat}
if (randomNum > currentWeightSum && randomNum <= (currentWeightSum +
currentValue.getWeight())) {
snapshotRandomOp = currentValue;
break;
}
{noformat}
Shouldn't the check be just (randomNum < (currentWeightSum +
currentValue.getWeight())
2.
{noformat}
private static MiniDFSCluster cluster;
private static DistributedFileSystem hdfs;
private static Random GENERATOR = null;
{noformat}
Above class members need not be static.
3.
{{FileSystemOperations}} and {{SnapshotOperations}} are very similar except for
enum values and weights. Code duplication here can be avoided if we can merge
these two enums into one and expose proper methods.
4.
{noformat}
// Set
Random RANDOM = new Random();
long seed = RANDOM.nextLong();
GENERATOR = new Random(seed);
{noformat}
Any specific reason why a simple seed like System.currentTimeMillis() will not
be useful here ? Here seed is generated from random, which is inturn is not
seeded. Also, RANDOM need not be all caps.
5.
{noformat}
int fileLen = new Random().nextInt(MAX_NUM_FILE_LENGTH);
createFiles(testDirString, fileLen);
{noformat}
GENERATOR random can be used here instead of creating a new one.
6.
{noformat}
// Create files in a directory with random depth, ranging from 0-10.
for (int i = 0; i < TOTAL_BLOCKS; i += fileLength) {
{noformat}
Is this TOTAL_BLOCKS are total files ?
7.
{noformat}
private String GetNewPathString(String originalString,
{noformat}
Metnhod name should be in camel case, like getNewPathString()
> Add a snapshot unit test with randomized file IO operations
> -----------------------------------------------------------
>
> Key: HDFS-11912
> URL: https://issues.apache.org/jira/browse/HDFS-11912
> Project: Hadoop HDFS
> Issue Type: Test
> Components: hdfs
> Reporter: George Huang
> Priority: Minor
> Attachments: HDFS-11912.001.patch
>
>
> Adding a snapshot unit test with randomized file IO operations.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
