[
https://issues.apache.org/jira/browse/HDDS-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16551082#comment-16551082
]
Nanda kumar commented on HDDS-249:
----------------------------------
Thanks [~bharatviswa] for working on this, the patch looks pretty good to me.
Some nitpicks:
*ContainerReader.java*
Line 56 & 57 can be put in same line.
*HddsVolumeUtil.java*
Line 198 & 190: {{hddsVolume.getHddsRootDir()}} can be replaced with
{{hddsRoot}}
Possible NullPointerException: Line:216 {{dnScmDir[0].getName()}}, this can
throw NPE if HddsRootDir doesn't contain any directory but two files.
In the below code, you don't need to have {{else}} structure as there is
{{return}} statement inside {{if}}
{code:java}
if(hddsFiles == null) {
// This is the case for IOException, where listFiles returns null.
// So, we fail the volume.
return false;
} else if (hddsFiles.length == 1) {
// DN started for first time or this is a newly added volume.
// So we create scm directory.
if (!scmDir.mkdir()) {
logger.error("Unable to create scmDir {}", scmDir);
return false;
}
return true;
} else if(hddsFiles.length == 2) {
// The files should be Version and SCM directory
if (scmDir.exists()) {
return true;
} else {
logger.error("Volume {} is in Inconsistent state, expected scm " +
"directory is {}, but available scm directory on datanode is " +
"{}", volumeRoot, scmId, dnScmDir[0].getName());
return false;
}
} else {
// The hdds root dir should always have 2 files. One is Version file
// and other is SCM directory.
return false;
}
{code}
For clarity, {{checkVolume}} method can be refactored to
{code:java}
public static boolean checkVolume(HddsVolume hddsVolume, String scmId, String
clusterId, Logger logger) {
File hddsRoot = hddsVolume.getHddsRootDir();
File scmDir = new File(hddsRoot, scmId);
try {
hddsVolume.format(clusterId);
File[] hddsFiles = hddsRoot.listFiles();
if (hddsFiles != null) {
if (hddsFiles.length == 1) {
if (scmDir.mkdir()) {
return true;
}
logger.error("Unable to create scmDir {}", scmDir);
}
if (hddsFiles.length == 2 && scmDir.exists()) {
return true;
}
logger.error("Volume {} is in Inconsistent state, expected 'VERSION'" +
" file and '{}' directory, but found: {} ", hddsRoot, scmId,
Arrays.asList(hddsFiles));
}
} catch (IOException ex) {
logger.error("Error during formatting volume {}, exception is {}",
hddsRoot, ex);
}
return false;
}
{code}
> Fail if multiple SCM IDs on the DataNode and add SCM ID check after version
> request
> -----------------------------------------------------------------------------------
>
> Key: HDDS-249
> URL: https://issues.apache.org/jira/browse/HDDS-249
> Project: Hadoop Distributed Data Store
> Issue Type: Improvement
> Reporter: Bharat Viswanadham
> Assignee: Bharat Viswanadham
> Priority: Major
> Fix For: 0.2.1
>
> Attachments: HDDS-249.00.patch, HDDS-249.01.patch, HDDS-249.02.patch,
> HDDS-249.03.patch, HDDS-249.04.patch, HDDS-249.05.patch, HDDS-249.06.patch,
> HDDS-249.07.patch
>
>
> This Jira take care of following conditions:
> # If multiple Scm directories exist on datanode, it fails that volume.
> # validate SCMID response from SCM.
--
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
