hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Manoj Govindassamy (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HDFS-11912) Add a snapshot unit test with randomized file IO operations
Date Thu, 01 Jun 2017 22:28:04 GMT

    [ 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-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-help@hadoop.apache.org


Mime
View raw message