wang-jiahua commented on code in PR #10514:
URL: https://github.com/apache/rocketmq/pull/10514#discussion_r3510568003
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java:
##########
@@ -509,6 +529,29 @@ public ByteBuffer encodeHeader(final int bodyLength) {
return result;
}
+ public ByteBuffer fastEncodeHeaderAsBuffer(final int bodyLength) {
Review Comment:
Removed from this PR. The `fastEncodeHeaderAsBuffer` method was not wired
into any transfer path, so it has been left out to avoid adding unused public
surface.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java:
##########
@@ -635,12 +681,17 @@ public void setSerializeTypeCurrentRPC(SerializeType
serializeTypeCurrentRPC) {
this.serializeTypeCurrentRPC = serializeTypeCurrentRPC;
}
- public Stopwatch getProcessTimer() {
- return processTimer;
+ public long processTimerElapsedMs() {
Review Comment:
Fixed — `processTimerElapsedMs()` now guards against `processTimerNanos ==
0` and returns 0 in that case, so unset commands don't report JVM uptime as
request latency.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java:
##########
@@ -635,12 +681,17 @@ public void setSerializeTypeCurrentRPC(SerializeType
serializeTypeCurrentRPC) {
this.serializeTypeCurrentRPC = serializeTypeCurrentRPC;
}
- public Stopwatch getProcessTimer() {
- return processTimer;
+ public long processTimerElapsedMs() {
+ return (System.nanoTime() - processTimerNanos) / 1_000_000;
+ }
+
+ @Deprecated
+ public com.google.common.base.Stopwatch getProcessTimer() {
+ return com.google.common.base.Stopwatch.createStarted();
Review Comment:
Added explicit `@deprecated` Javadoc documenting the semantic break: this
method returns a newly-started Stopwatch for source compatibility only and does
NOT represent the original request processing timer. All internal callers have
been migrated to `processTimerElapsedMs()`.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/statictopic/TopicQueueMappingContext.java:
##########
@@ -20,6 +20,8 @@
import java.util.List;
public class TopicQueueMappingContext {
+ public static final TopicQueueMappingContext EMPTY = new
TopicQueueMappingContext(null, null, null, null, null);
Review Comment:
Removed from this PR. The `TopicQueueMappingContext.EMPTY` singleton was
removed since the class is not truly immutable (has setters).
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RemotingCommand.java:
##########
@@ -635,12 +658,20 @@ public void setSerializeTypeCurrentRPC(SerializeType
serializeTypeCurrentRPC) {
this.serializeTypeCurrentRPC = serializeTypeCurrentRPC;
}
- public Stopwatch getProcessTimer() {
- return processTimer;
+ public long processTimerElapsedMs() {
+ if (processTimerNanos == 0) {
+ return 0;
+ }
+ return (System.nanoTime() - processTimerNanos) / 1_000_000;
+ }
+
+ @Deprecated
+ public com.google.common.base.Stopwatch getProcessTimer() {
Review Comment:
Same as above — added explicit `@deprecated` Javadoc on `getProcessTimer()`
documenting the semantic break. The `setProcessTimer(Stopwatch)` method is also
kept and deprecated, backed by `processTimerNanos`.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingServer.java:
##########
@@ -581,13 +581,17 @@ public void
channelWritabilityChanged(ChannelHandlerContext ctx) throws Exceptio
if (channel.isWritable()) {
if (!channel.config().isAutoRead()) {
channel.config().setAutoRead(true);
- log.info("Channel[{}] turns writable, bytes to buffer
before changing channel to un-writable: {}",
- RemotingHelper.parseChannelRemoteAddr(channel),
channel.bytesBeforeUnwritable());
+ if (log.isDebugEnabled()) {
+ log.debug("Channel[{}] turns writable, bytes to buffer
before changing channel to un-writable: {}",
+ RemotingHelper.parseChannelRemoteAddr(channel),
channel.bytesBeforeUnwritable());
+ }
}
} else {
channel.config().setAutoRead(false);
- log.warn("Channel[{}] auto-read is disabled, bytes to drain
before it turns writable: {}",
- RemotingHelper.parseChannelRemoteAddr(channel),
channel.bytesBeforeWritable());
+ if (log.isDebugEnabled()) {
+ log.debug("Channel[{}] auto-read is disabled, bytes to
drain before it turns writable: {}",
Review Comment:
Reverted to original log levels: `log.warn` for auto-read disabled
(backpressure), `log.info` for auto-read re-enabled. The downgrade to debug was
removed.
--
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]
