joeCarf commented on code in PR #7301:
URL: https://github.com/apache/rocketmq/pull/7301#discussion_r1343917297
##########
controller/src/main/java/org/apache/rocketmq/controller/Controller.java:
##########
@@ -17,21 +17,21 @@
package org.apache.rocketmq.controller;
-import java.util.List;
Review Comment:
seems that no need to change this file
##########
controller/src/main/java/org/apache/rocketmq/controller/ControllerManager.java:
##########
@@ -335,7 +352,8 @@ public int hashCode() {
@Override
public boolean equals(Object obj) {
- if (this == obj) return true;
+ if (this == obj)
Review Comment:
suggest use
``` java
if (this == obj) {
return true;
}
##########
controller/src/main/java/org/apache/rocketmq/controller/ControllerStartup.java:
##########
@@ -16,12 +16,6 @@
*/
package org.apache.rocketmq.controller;
-import java.io.BufferedInputStream;
Review Comment:
It's better to only format the code you change or add. It may cause
confusion for reviewers if formatting the
irrelevant file
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/BrokerReplicaInfo.java:
##########
@@ -101,7 +105,8 @@ public String getBrokerRegisterCheckCode(final Long
brokerId) {
}
public void updateBrokerAddress(final Long brokerId, final String
brokerAddress) {
- if (brokerId == null) return;
+ if (brokerId == null)
Review Comment:
same
##########
controller/src/main/java/org/apache/rocketmq/controller/ControllerStartup.java:
##########
@@ -16,12 +16,6 @@
*/
package org.apache.rocketmq.controller;
-import java.io.BufferedInputStream;
Review Comment:
I think It's hard to find your real changes in the file diff if you format
the whole file
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/BrokerReplicaInfo.java:
##########
@@ -83,7 +85,8 @@ public Map<Long, String> getBrokerIdTable() {
}
public String getBrokerAddress(final Long brokerId) {
- if (brokerId == null) return null;
+ if (brokerId == null)
Review Comment:
use braces
##########
controller/src/main/java/org/apache/rocketmq/controller/Controller.java:
##########
@@ -17,21 +17,21 @@
package org.apache.rocketmq.controller;
-import java.util.List;
Review Comment:
only format changes
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/DLedgerControllerStateMachine.java:
##########
@@ -21,14 +21,15 @@
import io.openmessaging.storage.dledger.snapshot.SnapshotWriter;
import io.openmessaging.storage.dledger.statemachine.CommittedEntryIterator;
import io.openmessaging.storage.dledger.statemachine.StateMachine;
-import java.util.concurrent.CompletableFuture;
import org.apache.rocketmq.common.constant.LoggerName;
import org.apache.rocketmq.controller.impl.event.EventMessage;
import org.apache.rocketmq.controller.impl.event.EventSerializer;
import org.apache.rocketmq.controller.impl.manager.ReplicasInfoManager;
import org.apache.rocketmq.logging.org.slf4j.Logger;
import org.apache.rocketmq.logging.org.slf4j.LoggerFactory;
+import java.util.concurrent.CompletableFuture;
Review Comment:
BTW, did you import the `rocketmq-code-style`, cause it's need not change in
my IDE
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/DLedgerController.java:
##########
@@ -303,7 +303,7 @@ private boolean appendToDLedgerAndWait(final
AppendEntryRequest request) {
attributesBuilder.put(LABEL_DLEDGER_OPERATION_STATUS,
ControllerMetricsConstant.DLedgerOperationStatus.FAILED.getLowerCaseName()).build());
return false;
}
- dLedgerFuture.get(5, TimeUnit.SECONDS);
+ dLedgerFuture.get(50, TimeUnit.SECONDS);
Review Comment:
Why increase this value
--
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]
