[jira] [Comment Edited] (HDDS-249) Fail if multiple SCM IDs on the DataNode and add SCM ID check after version request

2018-07-20 Thread Bharat Viswanadham (JIRA)


[ 
https://issues.apache.org/jira/browse/HDDS-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16551108#comment-16551108
 ] 

Bharat Viswanadham edited comment on HDDS-249 at 7/20/18 6:41 PM:
--

Hi [~nandakumar131]

Thanks for review.
{quote}Line 198 & 190: {{hddsVolume.getHddsRootDir()}} can be replaced with 
{{hddsRoot}}
{quote}
Addressed it.
{quote}Possible NullPointerException: Line:216 {{dnScmDir[0].getName()}}, this 
can throw NPE if HddsRootDir doesn't contain any directory but two files.
{quote}
Addressed in patch v07.

 
{quote}In the below code, you don't need to have {{else}} structure as there is 
{{return}} statement inside {{if}}
{quote}
This is added because if any volume in hddsRootDir has greater than 2 files we 
want to fail that volume, as hddsRoot should always have Version and Scm dir. 

For checkVolume, I just retained the old code.

 

Attached patch v08.

 

 

 

 

 


was (Author: bharatviswa):
Hi [~nandakumar131]

Thanks for review.
{quote}Line 198 & 190: {{hddsVolume.getHddsRootDir()}} can be replaced with 
{{hddsRoot}}
{quote}
Addressed it.
{quote}Possible NullPointerException: Line:216 {{dnScmDir[0].getName()}}, this 
can throw NPE if HddsRootDir doesn't contain any directory but two files.
{quote}
Addressed in patch v07.

 
{quote}In the below code, you don't need to have {{else}} structure as there is 
{{return}} statement inside {{if}}
{quote}
This is added because if any volume in hddsRootDir has greater than 2 files we 
want to fail that volume, as hddsRoot should always have Version and Scm dir. 
{quote} 
if (hddsFiles.length == 1) { if (scmDir.mkdir())

{ return true; }

logger.error("Unable to create scmDir {}", scmDir);
}
This code will not be true, if hddsFiles length is 1, as we just formatted it 
the file will be version file. So, this check should not be here.
{quote}
So, for checkVolume, I just retained the old code.

 

Attached patch v08.

 

 

 

 

 

> 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, HDDS-249.08.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



[jira] [Comment Edited] (HDDS-249) Fail if multiple SCM IDs on the DataNode and add SCM ID check after version request

2018-07-20 Thread Bharat Viswanadham (JIRA)


[ 
https://issues.apache.org/jira/browse/HDDS-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16551108#comment-16551108
 ] 

Bharat Viswanadham edited comment on HDDS-249 at 7/20/18 6:33 PM:
--

Hi [~nandakumar131]

Thanks for review.
{quote}Line 198 & 190: {{hddsVolume.getHddsRootDir()}} can be replaced with 
{{hddsRoot}}
{quote}
Addressed it.
{quote}Possible NullPointerException: Line:216 {{dnScmDir[0].getName()}}, this 
can throw NPE if HddsRootDir doesn't contain any directory but two files.
{quote}
Addressed in patch v07.

 
{quote}In the below code, you don't need to have {{else}} structure as there is 
{{return}} statement inside {{if}}
{quote}
This is added because if any volume in hddsRootDir has greater than 2 files we 
want to fail that volume, as hddsRoot should always have Version and Scm dir. 
{quote} 
if (hddsFiles.length == 1) { if (scmDir.mkdir())

{ return true; }

logger.error("Unable to create scmDir {}", scmDir);
}
This code will not be true, if hddsFiles length is 1, as we just formatted it 
the file will be version file. So, this check should not be here.
{quote}
So, for checkVolume, I just retained the old code.

 

Attached patch v08.

 

 

 

 

 


was (Author: bharatviswa):
Hi [~nandakumar131]

Thanks for review.

Line 198 & 190: {{hddsVolume.getHddsRootDir()}} can be replaced with 
{{hddsRoot}}

Addressed it.

 

Addressed in patch v07.

 

In the below code, you don't need to have {{else}} structure as there is 
{{return}} statement inside {{if}}

This is added because if any volume in hddsRootDir has greater than 2 files we 
want to fail that volume, as hddsRoot should always have Version and Scm dir. 

 
if (hddsFiles.length == 1) {  if (scmDir.mkdir()) {return 
true;
  }
  logger.error("Unable to create scmDir {}", scmDir);
}
This code will not be true, if hddsFiles length is 1, as we just formatted it 
the file will be version file. So, this check should not be here.

So, for checkVolume, I just retained the old code.

 

Attached patch v08.

 

 

 

 

 

> 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, HDDS-249.08.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



[jira] [Comment Edited] (HDDS-249) Fail if multiple SCM IDs on the DataNode and add SCM ID check after version request

2018-07-12 Thread Bharat Viswanadham (JIRA)


[ 
https://issues.apache.org/jira/browse/HDDS-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16542289#comment-16542289
 ] 

Bharat Viswanadham edited comment on HDDS-249 at 7/12/18 10:54 PM:
---

Hi [~hanishakoneru]

Thanks for the review.
 # before calling hddsVolume.getHddsRootDir().listFiles(), there is call for 
format which creates a version file inside hddsDir.  Consider a case format is 
succeded, the above logic check is fine for us, to safeguard we can add a check 
for the name of the file is version file. Consider the format failed to create 
VersionFile, then we return and consider that volume is failed. Because in that 
case, we return false. below is the code for that. if we return false, we add 
to failVolumeMap.

{code:java}
hddsVolume.format(clusterId);
} catch (IOException ex) {
logger.error("Error during formatting volume {}, exception is",
volumeRoot, ex);
return result;
}{code}
2. scmDir check is done here with file exists. So, that scmId matching check is 
not needed here, if above logic holds good.                                     
                                                                                
                                    
{code:java}
else if (!scmDir.exists()) {
// Already existing volume, and this is not first time dn is started
logger.error("Volume {} is in Inconsistent state, missing scm {} " +
"directory", volumeRoot, scmId);
}{code}


was (Author: bharatviswa):
Hi [~hanishakoneru]

Thanks for the review.
 # before calling hddsVolume.getHddsRootDir().listFiles(), there is call for 
format which creates a version file inside hddsDir.  So consider it is 
succeded, the above logic check is fine for us, to safeguard we can add a check 
for name of the file is version file. Consider the format failed to create 
VersionFile, then we return and consider that volume is failed. Because in that 
case, we return false. below is the code for that. if we return false, we add 
to failVolumeMap.

{code:java}
hddsVolume.format(clusterId);
} catch (IOException ex) {
logger.error("Error during formatting volume {}, exception is",
volumeRoot, ex);
return result;
}{code}
2. scmDir check is done here with file exists. So, that scmId matching check is 
not needed here, if above logic holds good.                                     
                                                                                
                                    
{code:java}
else if (!scmDir.exists()) {
// Already existing volume, and this is not first time dn is started
logger.error("Volume {} is in Inconsistent state, missing scm {} " +
"directory", volumeRoot, scmId);
}{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
>
>
> 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