Copilot commented on code in PR #10568:
URL: https://github.com/apache/rocketmq/pull/10568#discussion_r3503200810
##########
client/src/main/java/org/apache/rocketmq/client/ClientConfig.java:
##########
@@ -105,6 +105,12 @@ public class ClientConfig {
private int concurrentHeartbeatThreadPoolSize =
Runtime.getRuntime().availableProcessors();
+ /**
+ * When true, corrupted local offset files are silently ignored instead of
throwing an exception.
+ * The consumer starts with an empty offset table and falls back to
CONSUME_FROM_TIMESTAMP or broker-side offset.
+ */
Review Comment:
The Javadoc says corrupted local offset files are "silently ignored", but
the implementation logs a WARN (with stack trace) when ignoring. Please adjust
the wording to avoid a misleading contract.
##########
client/src/main/java/org/apache/rocketmq/client/ClientConfig.java:
##########
@@ -549,6 +555,14 @@ public void setConcurrentHeartbeatThreadPoolSize(int
concurrentHeartbeatThreadPo
this.concurrentHeartbeatThreadPoolSize =
concurrentHeartbeatThreadPoolSize;
}
+ public boolean isLocalOffsetStoreIgnoreCorrupted() {
+ return localOffsetStoreIgnoreCorrupted;
+ }
+
+ public void setLocalOffsetStoreIgnoreCorrupted(boolean
localOffsetStoreIgnoreCorrupted) {
Review Comment:
This new flag is not propagated by ClientConfig.cloneClientConfig() and
resetClientConfig(...). MQClientManager creates MQClientInstance with
clientConfig.cloneClientConfig(), so user-configuring
localOffsetStoreIgnoreCorrupted=true will be lost and the ignore behavior won’t
take effect. Ensure both cloneClientConfig() and resetClientConfig(...) copy
this field.
##########
client/src/main/java/org/apache/rocketmq/client/consumer/store/LocalFileOffsetStore.java:
##########
@@ -263,6 +263,10 @@ private OffsetSerializeWrapper readLocalOffsetBak() throws
MQClientException {
offsetSerializeWrapper =
OffsetSerializeWrapper.fromJson(content,
OffsetSerializeWrapper.class);
} catch (Exception e) {
+ if
(this.mQClientFactory.getClientConfig().isLocalOffsetStoreIgnoreCorrupted()) {
+ log.warn("readLocalOffsetBak: local offset file corrupted,
ignoring per config. group={}", this.groupName, e);
+ return null;
Review Comment:
The new ignoreCorrupted behavior isn’t covered by tests. There is an
existing LocalFileOffsetStoreTest; please add a unit test that writes invalid
JSON to offsets.json.bak, sets
ClientConfig.localOffsetStoreIgnoreCorrupted=true on the mocked
MQClientInstance, and verifies load()/readOffset do not throw and the offset
table starts empty.
--
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]
