-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-25888 Backup tests are categorically flakey #3279
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
HBASE-25888 Backup tests are categorically flakey #3279
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
0604f22
to
426db9c
Compare
426db9c
to
8048d75
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@@ -240,7 +240,7 @@ protected void copyMetaData(FileSystem fs, Path tmpBackupDir, Path backupDirPath | |||
* @throws IOException exception | |||
*/ | |||
protected void copyFile(FileSystem fs, Path p, Path newPath) throws IOException { | |||
File f = File.createTempFile("data", "meta"); | |||
File f = File.createTempFile("data", "meta", new File(System.getProperty("java.io.tmpdir"))); |
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.
Why do we need to download the whole file?
Just create an InputStream and an OutputStream and then use IOUtils.copy?
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.
@Apache9 Not sure if I understood what you meant. Can you explain a bit more detail?
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.
Sure. I got your point. let me try to fix it.
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.
@Apache9 Addressed your concern.
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
try (InputStream in = fs.open(p); | ||
OutputStream out = fs.create(newPath, true)) { | ||
IOUtils.copy(in, out); | ||
boolean exists = fs.exists(newPath); |
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.
Please move this test outside the try block, as till here, we haven't close the outout yet...
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.
missed this. my bad.
Fixed.
608aeed
to
0e4bf46
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Please fix the checkstyle and javac issues? Seems easy to fix. Thanks. |
Anyd document to refer to? |
Please see the above comments made by robot. There are two -0 items. https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3279/5/artifact/yetus-general-check/output/diff-compile-javac-hbase-backup.txt |
Will check and fix. Thanks |
0e4bf46
to
57e9587
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Addressed. |
No description provided.