You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "sharmaar12 (via GitHub)" <gi...@apache.org> on 2026/03/07 10:14:34 UTC
[PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
sharmaar12 opened a new pull request, #7881:
URL: https://github.com/apache/hbase/pull/7881
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
Posted by "sharmaar12 (via GitHub)" <gi...@apache.org>.
sharmaar12 commented on code in PR #7881:
URL: https://github.com/apache/hbase/pull/7881#discussion_r2906915218
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java:
##########
@@ -61,7 +64,12 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile
activeClusterFile);
try {
if (fs.exists(activeClusterFile)) {
- fs.delete(activeClusterFile, false);
+ FSDataInputStream in = fs.open(activeClusterFile);
+ byte[] actualClusterFileData = IOUtils.toByteArray(in);
+ byte[] expectedClusterFileData = mfs.getSuffixFileDataToWrite();
+ if (Arrays.equals(actualClusterFileData, expectedClusterFileData)) {
+ fs.delete(activeClusterFile, false);
Review Comment:
Done. Thanks for pointing it out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #7881:
URL: https://github.com/apache/hbase/pull/7881#discussion_r2908164981
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java:
##########
@@ -61,8 +64,13 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile
activeClusterFile);
try {
if (fs.exists(activeClusterFile)) {
- fs.delete(activeClusterFile, false);
- LOG.info("Successfully deleted active cluster file: {}", activeClusterFile);
+ FSDataInputStream in = fs.open(activeClusterFile);
+ byte[] actualClusterFileData = IOUtils.toByteArray(in);
+ byte[] expectedClusterFileData = mfs.getSuffixFileDataToWrite();
Review Comment:
Another nice catch. Getters should not mutate anything. Let's extract the mutation and move it to callers' level.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
Posted by "sharmaar12 (via GitHub)" <gi...@apache.org>.
sharmaar12 commented on code in PR #7881:
URL: https://github.com/apache/hbase/pull/7881#discussion_r2911979009
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java:
##########
@@ -61,8 +64,13 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile
activeClusterFile);
try {
if (fs.exists(activeClusterFile)) {
- fs.delete(activeClusterFile, false);
- LOG.info("Successfully deleted active cluster file: {}", activeClusterFile);
+ FSDataInputStream in = fs.open(activeClusterFile);
Review Comment:
Done. Thanks for pointing out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #7881:
URL: https://github.com/apache/hbase/pull/7881#discussion_r2906545269
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java:
##########
@@ -61,7 +64,12 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile
activeClusterFile);
try {
if (fs.exists(activeClusterFile)) {
- fs.delete(activeClusterFile, false);
+ FSDataInputStream in = fs.open(activeClusterFile);
+ byte[] actualClusterFileData = IOUtils.toByteArray(in);
+ byte[] expectedClusterFileData = mfs.getSuffixFileDataToWrite();
+ if (Arrays.equals(actualClusterFileData, expectedClusterFileData)) {
+ fs.delete(activeClusterFile, false);
Review Comment:
The LOG message should be inside the IF branch, because that's the case when you actually delete the file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
Posted by "Kota-SH (via GitHub)" <gi...@apache.org>.
Kota-SH commented on code in PR #7881:
URL: https://github.com/apache/hbase/pull/7881#discussion_r2907056300
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java:
##########
@@ -61,8 +64,13 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile
activeClusterFile);
try {
if (fs.exists(activeClusterFile)) {
- fs.delete(activeClusterFile, false);
- LOG.info("Successfully deleted active cluster file: {}", activeClusterFile);
+ FSDataInputStream in = fs.open(activeClusterFile);
Review Comment:
`FSDataInputStream in` is opened but never closed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
Posted by "Kota-SH (via GitHub)" <gi...@apache.org>.
Kota-SH commented on code in PR #7881:
URL: https://github.com/apache/hbase/pull/7881#discussion_r2907834910
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java:
##########
@@ -61,8 +64,13 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile
activeClusterFile);
try {
if (fs.exists(activeClusterFile)) {
- fs.delete(activeClusterFile, false);
- LOG.info("Successfully deleted active cluster file: {}", activeClusterFile);
+ FSDataInputStream in = fs.open(activeClusterFile);
+ byte[] actualClusterFileData = IOUtils.toByteArray(in);
+ byte[] expectedClusterFileData = mfs.getSuffixFileDataToWrite();
Review 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);
> }
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
Posted by "sharmaar12 (via GitHub)" <gi...@apache.org>.
sharmaar12 commented on code in PR #7881:
URL: https://github.com/apache/hbase/pull/7881#discussion_r2913067923
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java:
##########
@@ -61,8 +64,13 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile
activeClusterFile);
try {
if (fs.exists(activeClusterFile)) {
- fs.delete(activeClusterFile, false);
- LOG.info("Successfully deleted active cluster file: {}", activeClusterFile);
+ FSDataInputStream in = fs.open(activeClusterFile);
+ byte[] actualClusterFileData = IOUtils.toByteArray(in);
+ byte[] expectedClusterFileData = mfs.getSuffixFileDataToWrite();
Review Comment:
Thanks @Kota-SH , I will be using the same method to not update the activeClusterSuffix variable unintentionally.
Also note that instead of get I am changing the name to computeAndSetSuffixFileDataToWrite() instead of getSuffixFileDataToWrite()
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
Posted by "Kota-SH (via GitHub)" <gi...@apache.org>.
Kota-SH commented on code in PR #7881:
URL: https://github.com/apache/hbase/pull/7881#discussion_r2912469245
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java:
##########
@@ -61,8 +64,13 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile
activeClusterFile);
try {
if (fs.exists(activeClusterFile)) {
- fs.delete(activeClusterFile, false);
- LOG.info("Successfully deleted active cluster file: {}", activeClusterFile);
+ FSDataInputStream in = fs.open(activeClusterFile);
+ byte[] actualClusterFileData = IOUtils.toByteArray(in);
+ byte[] expectedClusterFileData = mfs.getSuffixFileDataToWrite();
Review Comment:
@sharmaar12, Could we use MasterFileSystem.java#getSuffixFromConfig() instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar merged PR #7881:
URL: https://github.com/apache/hbase/pull/7881
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29959 Cluster started in read-only mode mistakenly deletes suffix file during startup [hbase]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #7881:
URL: https://github.com/apache/hbase/pull/7881#discussion_r2907078613
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java:
##########
@@ -61,8 +64,13 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile
activeClusterFile);
try {
if (fs.exists(activeClusterFile)) {
- fs.delete(activeClusterFile, false);
- LOG.info("Successfully deleted active cluster file: {}", activeClusterFile);
+ FSDataInputStream in = fs.open(activeClusterFile);
Review Comment:
good catch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org