HDFS-16338. Correct fsimage error configuration message #3684
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -587,7 +587,7 @@ void doImportCheckpoint(FSNamesystem target) throws IOException { | |
|
|
||
| if (checkpointEditsDirs == null || checkpointEditsDirs.isEmpty()) { | ||
| throw new IOException("Cannot import image from a checkpoint. " | ||
| + "\"dfs.namenode.checkpoint.edits.dir\" is not set." ); | ||
|
There was a problem hiding this comment. Looks like there are some checktyles that need fixing. Maybe you can configure There was a problem hiding this comment. great! Thanks @tomscut for your advice, |
||
| } | ||
|
|
||
| FSImage realImage = target.getFSImage(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,6 +86,7 @@ | |
| import org.junit.Test; | ||
|
|
||
| import static org.junit.Assert.assertArrayEquals; | ||
| import static org.junit.Assert.fail; | ||
|
|
||
| public class TestFSImage { | ||
|
|
||
|
|
@@ -275,6 +276,29 @@ public void testSaveAndLoadStripedINodeFile() throws IOException{ | |
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testImportCheckpoint() throws IOException{ | ||
|
There was a problem hiding this comment. A little advice. You can consider using try with resources for There was a problem hiding this comment. Thanks @tomscut for your advice , it seems simpler code. |
||
| Configuration conf = new Configuration(); | ||
| conf.set(DFSConfigKeys.DFS_NAMENODE_CHECKPOINT_EDITS_DIR_KEY,""); | ||
| MiniDFSCluster cluster = null; | ||
| try { | ||
GuoPhilipse marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| cluster = new MiniDFSCluster.Builder(conf).build(); | ||
| cluster.waitActive(); | ||
| FSNamesystem fsn = cluster.getNamesystem(); | ||
| FSImage fsImage= new FSImage(conf); | ||
| fsImage.doImportCheckpoint(fsn); | ||
| fail("Expect to throw IOException."); | ||
| } catch (IOException e) { | ||
| GenericTestUtils.assertExceptionContains( | ||
| "Cannot import image from a checkpoint. " | ||
| + "\"dfs.namenode.checkpoint.edits.dir\" is not set.", e); | ||
|
There was a problem hiding this comment. Use LambdaTestUtils instead of try-catch-assert There was a problem hiding this comment. Thanks @ayushtkn for your advice, just updated. |
||
| } finally { | ||
| if (cluster != null) { | ||
| cluster.shutdown(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Test if a INodeFileUnderConstruction with BlockInfoStriped can be | ||
| * saved and loaded by FSImageSerialization | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw
checkpointDirsandcheckpointEditsDirswill not be null as they are assigned values returned byFSImage.getCheckpointDirsandFSImage.getCheckpointEditsDirsrespectively.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, my IntelliJ was giving that warning as well, but that was something not touched here....
I thought Todd added it just for future safety, Didn't research why there was a null check considering it harmless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it seems fine, just an additional null check, not a big deal. Also good to keep it for future safety as you predicted correctly, just in case if the methods themselves start returning null (less likely but still doable).