[GitHub] [hbase] saintstack commented on a change in pull request #544: HBASE-22917 Proc-WAL roll fails saying someone else has already created log
saintstack commented on a change in pull request #544: HBASE-22917 Proc-WAL
roll fails saying someone else has already created log
URL: https://github.com/apache/hbase/pull/544#discussion_r319814693
##
File path:
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java
##
@@ -1100,7 +1100,25 @@ boolean rollWriter(long logId) throws IOException {
startPos = newStream.getPos();
} catch (IOException ioe) {
LOG.warn("Encountered exception writing header", ioe);
- newStream.close();
+ try {
+newStream.close();
+ } catch (IOException e) {
+LOG.error("Exception occured while closing the file {}", newLogFile,
e);
+ }
+ try {
+// Delete the incomplete file
+if (!fs.delete(newLogFile, false)) {
+ LOG.warn(
+"Failed to delete the log file {}, increasing the log id by 1 for
the next roll attempt",
+newLogFile);
+ flushLogId++;
+}
+ } catch (IOException e) {
+LOG.warn("Exception occured while deleting the file {}", newLogFile,
e);
+flushLogId++;
+LOG.info("Increased the log id to {}", flushLogId);
+ }
+
Review comment:
I think I said this would be easier to read if out in a standalone method.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #544: HBASE-22917 Proc-WAL roll fails saying someone else has already created log
saintstack commented on a change in pull request #544: HBASE-22917 Proc-WAL
roll fails saying someone else has already created log
URL: https://github.com/apache/hbase/pull/544#discussion_r319814312
##
File path:
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java
##
@@ -423,7 +423,7 @@ public void recoverLease() throws IOException {
// Create new state-log
if (!rollWriter(flushLogId + 1)) {
// someone else has already created this log
- LOG.debug("Someone else has already created log {}. Retrying.",
flushLogId);
+ LOG.debug("Someone else has already created log {}. Retrying.",
flushLogId + 1);
Review comment:
I had a review done a while ago but forgot to submit then I lost it!
IIRC, suggested doing the flushLogId + 1 into a new variable and then using
that rather than increment in two places. IIRC, there is another place after
this that needs the +1 done to it too.. or what we log is flushLogId w/o +1
when it should have +1.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #544: HBASE-22917 Proc-WAL roll fails saying someone else has already created log
saintstack commented on a change in pull request #544: HBASE-22917 Proc-WAL
roll fails saying someone else has already created log
URL: https://github.com/apache/hbase/pull/544#discussion_r324431017
##
File path:
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java
##
@@ -1100,7 +1100,25 @@ boolean rollWriter(long logId) throws IOException {
startPos = newStream.getPos();
} catch (IOException ioe) {
LOG.warn("Encountered exception writing header", ioe);
- newStream.close();
+ try {
+newStream.close();
+ } catch (IOException e) {
+LOG.error("Exception occured while closing the file {}", newLogFile,
e);
+ }
+ try {
+// Delete the incomplete file
+if (!fs.delete(newLogFile, false)) {
+ LOG.warn(
+"Failed to delete the log file {}, increasing the log id by 1 for
the next roll attempt",
+newLogFile);
+ flushLogId++;
+}
+ } catch (IOException e) {
+LOG.warn("Exception occured while deleting the file {}", newLogFile,
e);
+flushLogId++;
+LOG.info("Increased the log id to {}", flushLogId);
+ }
+
Review comment:
And it looks like you have a good comeback on this concern above going by
your response to Duo?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #544: HBASE-22917 Proc-WAL roll fails saying someone else has already created log
saintstack commented on a change in pull request #544: HBASE-22917 Proc-WAL
roll fails saying someone else has already created log
URL: https://github.com/apache/hbase/pull/544#discussion_r319814981
##
File path:
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java
##
@@ -1100,7 +1100,25 @@ boolean rollWriter(long logId) throws IOException {
startPos = newStream.getPos();
} catch (IOException ioe) {
LOG.warn("Encountered exception writing header", ioe);
- newStream.close();
+ try {
+newStream.close();
+ } catch (IOException e) {
+LOG.error("Exception occured while closing the file {}", newLogFile,
e);
+ }
+ try {
+// Delete the incomplete file
+if (!fs.delete(newLogFile, false)) {
+ LOG.warn(
+"Failed to delete the log file {}, increasing the log id by 1 for
the next roll attempt",
+newLogFile);
+ flushLogId++;
+}
+ } catch (IOException e) {
+LOG.warn("Exception occured while deleting the file {}", newLogFile,
e);
+flushLogId++;
+LOG.info("Increased the log id to {}", flushLogId);
+ }
+
Review comment:
And yeah, the close and delete would be the wrong thing to do if this is a
dead server but doesn't know it yet (and another has taken over WAL writing).
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #544: HBASE-22917 Proc-WAL roll fails saying someone else has already created log
saintstack commented on a change in pull request #544: HBASE-22917 Proc-WAL
roll fails saying someone else has already created log
URL: https://github.com/apache/hbase/pull/544#discussion_r324430998
##
File path:
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java
##
@@ -423,7 +423,7 @@ public void recoverLease() throws IOException {
// Create new state-log
if (!rollWriter(flushLogId + 1)) {
// someone else has already created this log
- LOG.debug("Someone else has already created log {}. Retrying.",
flushLogId);
+ LOG.debug("Someone else has already created log {}. Retrying.",
flushLogId + 1);
Review comment:
What you think of above @pankaj72981 ?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
