-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDFS-16338. Correct fsimage error configuration message #3684
Changes from 15 commits
3c51579
a449d04
d4f968b
d522abf
de633b8
66ce06b
c0aa861
4743a6a
bea061f
ad9eab5
ef4d239
95909ec
a883029
aa0bb69
5072db4
faf3055
f748f13
f05f24a
f70eddb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
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.dir\" is not set." ); | ||
+ "\"dfs.namenode.checkpoint.edits.dir\" is not set." ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like there are some checktyles that need fixing. Maybe you can configure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A little advice. You can consider using try with resources for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use LambdaTestUtils instead of try-catch-assert There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
checkpointDirs
andcheckpointEditsDirs
will not be null as they are assigned values returned byFSImage.getCheckpointDirs
andFSImage.getCheckpointEditsDirs
respectively.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).