Skip to content

Commit 36adf12

Browse files
guyinyouguyinyou
andauthored
[ISSUE #10017] Validate commitlog offset in recoverAbnormally to prevent processing … (#10018)
* validate commitlog offset in recoverAbnormally to prevent processing old file data that passes CRC checks Change-Id: If4b1881f82d26ce8d374472d73ec9ce3d51ba643 * fix Change-Id: Idc4bf7ec476cc9b6529619c2aa9afd6a980b819c * add checkCommitLogOffsetOnRecover in MessageStoreConfig Change-Id: Iac9afbb8b3ffb03fa15890decaf502afbfa44cf9 --------- Co-authored-by: guyinyou <guyinyou.gyy@alibaba-inc.com>
1 parent 899a1b5 commit 36adf12

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed

store/src/main/java/org/apache/rocketmq/store/CommitLog.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,7 @@ public void recoverAbnormally(long maxPhyOffsetOfConsumeQueue) throws RocksDBExc
738738
// recover by the minimum time stamp
739739
boolean checkCRCOnRecover = this.defaultMessageStore.getMessageStoreConfig().isCheckCRCOnRecover();
740740
boolean checkDupInfo = this.defaultMessageStore.getMessageStoreConfig().isDuplicationEnable();
741+
boolean checkCommitLogOffsetOnRecover = this.defaultMessageStore.getMessageStoreConfig().isCheckCommitLogOffsetOnRecover();
741742
int maxRecoverNum = this.defaultMessageStore.getMessageStoreConfig().getCommitLogRecoverMaxNum();
742743
if (maxRecoverNum <= 0) {
743744
maxRecoverNum = 10;
@@ -792,8 +793,18 @@ public void recoverAbnormally(long maxPhyOffsetOfConsumeQueue) throws RocksDBExc
792793
while (true) {
793794
DispatchRequest dispatchRequest = this.checkMessageAndReturnSize(byteBuffer, checkCRCOnRecover, checkDupInfo);
794795
int size = dispatchRequest.getMsgSize();
795-
796796
if (dispatchRequest.isSuccess()) {
797+
// Check commitlog offset validity if enabled
798+
if (checkCommitLogOffsetOnRecover) {
799+
if (dispatchRequest.getCommitLogOffset() < mappedFile.getFileFromOffset()
800+
|| dispatchRequest.getCommitLogOffset() > mappedFile.getFileFromOffset() + mappedFile.getFileSize()) {
801+
log.warn("found illegal commitlog offset {} in {}, current pos={}, it will be truncated.",
802+
dispatchRequest.getCommitLogOffset(), mappedFile.getFileName(), processOffset + mappedFileOffset);
803+
log.info("recover physics file end, {} pos={}", mappedFile.getFileName(), byteBuffer.position());
804+
805+
break;
806+
}
807+
}
797808
// Normal data
798809
if (size > 0) {
799810
lastValidMsgPhyOffset = processOffset + mappedFileOffset;
@@ -925,7 +936,8 @@ private boolean isMappedFileMatchedRecover(final MappedFile mappedFile,
925936
return isMappedFileMatchedRecover(phyOffset, storeTimestamp, recoverNormally);
926937
}
927938

928-
private boolean isMappedFileMatchedRecover(long phyOffset, long storeTimestamp, boolean recoverNormally) throws RocksDBException {
939+
private boolean isMappedFileMatchedRecover(long phyOffset, long storeTimestamp,
940+
boolean recoverNormally) throws RocksDBException {
929941
boolean result = this.defaultMessageStore.getQueueStore().isMappedFileMatchedRecover(phyOffset, storeTimestamp, recoverNormally);
930942
if (null != this.defaultMessageStore.getTransMessageRocksDBStore() && defaultMessageStore.getMessageStoreConfig().isTransRocksDBEnable() && !defaultMessageStore.getMessageStoreConfig().isTransWriteOriginTransHalfEnable()) {
931943
result = result && this.defaultMessageStore.getTransMessageRocksDBStore().isMappedFileMatchedRecover(phyOffset);
@@ -2386,7 +2398,7 @@ public void run() {
23862398
long costTime = this.systemClock.now() - beginClockTimestamp;
23872399
log.info("[{}] scanFilesInPageCache-cost {} ms.", costTime > 30 * 1000 ? "NOTIFYME" : "OK", costTime);
23882400
} catch (Throwable e) {
2389-
log.warn("{} service has e: ", this.getServiceName() , e);
2401+
log.warn("{} service has e: ", this.getServiceName(), e);
23902402
}
23912403
}
23922404
log.info("{} service end", this.getServiceName());

store/src/main/java/org/apache/rocketmq/store/config/MessageStoreConfig.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,9 @@ public class MessageStoreConfig {
194194
// This ensures no on-the-wire or on-disk corruption to the messages occurred.
195195
// This check adds some overhead,so it may be disabled in cases seeking extreme performance.
196196
private boolean checkCRCOnRecover = true;
197+
// Whether check the commitlog offset validity during abnormal recovery.
198+
// This helps detect and truncate old file data that may pass CRC checks but contains invalid offsets.
199+
private boolean checkCommitLogOffsetOnRecover = false;
197200
// How many pages are to be flushed when flush CommitLog
198201
private int flushCommitLogLeastPages = 4;
199202
// How many pages are to be committed when commit data to file
@@ -796,6 +799,14 @@ public void setCheckCRCOnRecover(boolean checkCRCOnRecover) {
796799
this.checkCRCOnRecover = checkCRCOnRecover;
797800
}
798801

802+
public boolean isCheckCommitLogOffsetOnRecover() {
803+
return checkCommitLogOffsetOnRecover;
804+
}
805+
806+
public void setCheckCommitLogOffsetOnRecover(boolean checkCommitLogOffsetOnRecover) {
807+
this.checkCommitLogOffsetOnRecover = checkCommitLogOffsetOnRecover;
808+
}
809+
799810
public boolean isForceVerifyPropCRC() {
800811
return forceVerifyPropCRC;
801812
}

0 commit comments

Comments
 (0)