[jira] [Comment Edited] (HDDS-249) Fail if multiple SCM IDs on the DataNode and add SCM ID check after version request
[ 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
[ 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
[ 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