HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup#7881
HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup#7881sharmaar12 wants to merge 2 commits intoapache:HBASE-29081from
Conversation
…ix file during startup
anmolnar
left a comment
anmolnar
left a comment
There was a problem hiding this comment.
Please fix the logging issue, otherwise lgtm.
| byte[] actualClusterFileData = IOUtils.toByteArray(in); | ||
| byte[] expectedClusterFileData = mfs.getSuffixFileDataToWrite(); | ||
| if (Arrays.equals(actualClusterFileData, expectedClusterFileData)) { | ||
| fs.delete(activeClusterFile, false); |
There was a problem hiding this comment.
The LOG message should be inside the IF branch, because that's the case when you actually delete the file.
There was a problem hiding this comment.
Done. Thanks for pointing it out.
| if (fs.exists(activeClusterFile)) { | ||
| fs.delete(activeClusterFile, false); | ||
| LOG.info("Successfully deleted active cluster file: {}", activeClusterFile); | ||
| FSDataInputStream in = fs.open(activeClusterFile); |
There was a problem hiding this comment.
FSDataInputStream in is opened but never closed.
| LOG.info("Successfully deleted active cluster file: {}", activeClusterFile); | ||
| FSDataInputStream in = fs.open(activeClusterFile); | ||
| byte[] actualClusterFileData = IOUtils.toByteArray(in); | ||
| byte[] expectedClusterFileData = mfs.getSuffixFileDataToWrite(); |
There was a problem hiding this comment.
getSuffixFileDataToWrite() mutates activeClusterSuffix in MasterFileSystem.java (line#453). Would this have any unintended side effects?
Because currently for read only clusters, this.activeClusterSuffix value is not set in MasterFileSystem.java but this PR changes it.
public byte[] getSuffixFileDataToWrite() {
String str = getClusterId().toString() + ":" + getActiveClusterSuffixFromConfig(conf);
this.activeClusterSuffix = new ActiveClusterSuffix(str);
return str.getBytes(StandardCharsets.UTF_8);
}
There was a problem hiding this comment.
Another nice catch. Getters should not mutate anything. Let's extract the mutation and move it to callers' level.
3 participants
No description provided.