Jason918 commented on a change in pull request #3382:
URL: https://github.com/apache/rocketmq/pull/3382#discussion_r721008192
##########
File path:
common/src/main/java/org/apache/rocketmq/common/protocol/ResponseCode.java
##########
@@ -20,7 +20,9 @@
import org.apache.rocketmq.remoting.protocol.RemotingSysResponseCode;
public class ResponseCode extends RemotingSysResponseCode {
-
+
+ public static final int FLUSH_DISK_FAILED = 9;
Review comment:
It's always better adding constant code after existing codes, not
before.
##########
File path: store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java
##########
@@ -442,6 +443,9 @@ public boolean flush(final int flushLeastPages) {
if (mappedFile != null) {
long tmpTimeStamp = mappedFile.getStoreTimestamp();
int offset = mappedFile.flush(flushLeastPages);
+ if (mappedFile.getflushError()) {
Review comment:
If we got error during flush, is it better not forwarding `flushedWhere`
below.?
##########
File path: store/src/main/java/org/apache/rocketmq/store/MappedFile.java
##########
@@ -288,6 +289,7 @@ public int flush(final int flushLeastPages) {
}
} catch (Throwable e) {
log.error("Error occurred when force data to disk.", e);
+ this.flushError = true;
Review comment:
Once the error occurs, the flushError will never reset, and the whole
broker is not writable, right?
##########
File path: store/src/main/java/org/apache/rocketmq/store/CommitLog.java
##########
@@ -1204,7 +1204,12 @@ private void doCommit() {
flushOK =
CommitLog.this.mappedFileQueue.getFlushedWhere() >= req.getNextOffset();
}
- req.wakeupCustomer(flushOK ? PutMessageStatus.PUT_OK :
PutMessageStatus.FLUSH_DISK_TIMEOUT);
+ if (CommitLog.this.mappedFileQueue.isFlushError()) {
+ req.wakeupCustomer(PutMessageStatus.FLUSH_DISK_FAILED);
Review comment:
The indentation seems not right. Please check the code format settings.
##########
File path: store/src/main/java/org/apache/rocketmq/store/CommitLog.java
##########
@@ -1161,7 +1161,7 @@ public void wakeupCustomer(final PutMessageStatus
putMessageStatus) {
public CompletableFuture<PutMessageStatus> future() {
return flushOKFuture;
}
-
+
Review comment:
Not necessary modification.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
