[GitHub] incubator-rocketmq pull request #150: [ROCKETMQ-273] return an expression wh...
Github user kevin-better commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/150#discussion_r139874392
--- Diff:
store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java ---
@@ -1094,34 +1090,15 @@ private boolean checkInDiskByCommitOffset(long
offsetPy, long maxOffsetPy) {
}
private boolean isTheBatchFull(int sizePy, int maxMsgNums, int
bufferTotal, int messageTotal, boolean isInDisk) {
-
-if (0 == bufferTotal || 0 == messageTotal) {
-return false;
-}
-
-if (maxMsgNums <= messageTotal) {
-return true;
-}
-
-if (isInDisk) {
-if ((bufferTotal + sizePy) >
this.messageStoreConfig.getMaxTransferBytesOnMessageInDisk()) {
-return true;
-}
-
-if (messageTotal >
this.messageStoreConfig.getMaxTransferCountOnMessageInDisk() - 1) {
-return true;
-}
-} else {
-if ((bufferTotal + sizePy) >
this.messageStoreConfig.getMaxTransferBytesOnMessageInMemory()) {
-return true;
-}
-
-if (messageTotal >
this.messageStoreConfig.getMaxTransferCountOnMessageInMemory() - 1) {
-return true;
-}
-}
-
-return false;
+return (0 != bufferTotal && 0 != messageTotal)
+&&(
--- End diff --
It has been restored
---
[GitHub] incubator-rocketmq pull request #166: doing some work
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/166 ---
[GitHub] incubator-rocketmq pull request #57: [ROCKETMQ-91] Reduce lock granularity f...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/57 ---
[GitHub] incubator-rocketmq pull request #81: [ROCKETMQ-117] add telnet server to nam...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/81 ---
[GitHub] incubator-rocketmq pull request #164: Fix tool's parameter
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/164 ---
[GitHub] incubator-rocketmq pull request #38: [ROCKETMQ-44] Refactor to avoid duplica...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/38 ---
[GitHub] incubator-rocketmq pull request #117: [ROCKETMQ-215]-Use java 7 syntax to re...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/117 ---
[GitHub] incubator-rocketmq pull request #99: Correct comment information
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/99 ---
[GitHub] incubator-rocketmq pull request #167: Merge pull request #1 from apache/mast...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/167 ---
[GitHub] incubator-rocketmq pull request #20: [ROCKETMQ-23] MappedFileQueue#flush sho...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/20 ---
[GitHub] incubator-rocketmq pull request #150: [ROCKETMQ-273] return an expression wh...
Github user vongosling commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/150#discussion_r139873296
--- Diff:
store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java ---
@@ -1094,34 +1090,15 @@ private boolean checkInDiskByCommitOffset(long
offsetPy, long maxOffsetPy) {
}
private boolean isTheBatchFull(int sizePy, int maxMsgNums, int
bufferTotal, int messageTotal, boolean isInDisk) {
-
-if (0 == bufferTotal || 0 == messageTotal) {
-return false;
-}
-
-if (maxMsgNums <= messageTotal) {
-return true;
-}
-
-if (isInDisk) {
-if ((bufferTotal + sizePy) >
this.messageStoreConfig.getMaxTransferBytesOnMessageInDisk()) {
-return true;
-}
-
-if (messageTotal >
this.messageStoreConfig.getMaxTransferCountOnMessageInDisk() - 1) {
-return true;
-}
-} else {
-if ((bufferTotal + sizePy) >
this.messageStoreConfig.getMaxTransferBytesOnMessageInMemory()) {
-return true;
-}
-
-if (messageTotal >
this.messageStoreConfig.getMaxTransferCountOnMessageInMemory() - 1) {
-return true;
-}
-}
-
-return false;
+return (0 != bufferTotal && 0 != messageTotal)
+&&(
--- End diff --
Human readable problem
---
[GitHub] incubator-rocketmq issue #150: [ROCKETMQ-273] return an expression when a fu...
Github user kevin-better commented on the issue: https://github.com/apache/incubator-rocketmq/pull/150 @dongeforever ok ---
[GitHub] incubator-rocketmq pull request #150: [ROCKETMQ-273] return an expression wh...
Github user kevin-better closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/150 ---
[GitHub] incubator-rocketmq pull request #152: [ROCKETMQ-278] Add clusterlist cmd by ...
Github user vongosling commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/152#discussion_r139871853
--- Diff:
namesrv/src/main/java/org/apache/rocketmq/namesrv/routeinfo/RouteInfoManager.java
---
@@ -63,11 +65,36 @@ public RouteInfoManager() {
this.filterServerTable = new HashMap(256);
}
-public byte[] getAllClusterInfo() {
-ClusterInfo clusterInfoSerializeWrapper = new ClusterInfo();
-
clusterInfoSerializeWrapper.setBrokerAddrTable(this.brokerAddrTable);
-
clusterInfoSerializeWrapper.setClusterAddrTable(this.clusterAddrTable);
-return clusterInfoSerializeWrapper.encode();
+public byte[] getAllClusterInfo(String cluster) {
--- End diff --
I agree @shroman here
---
[GitHub] incubator-rocketmq issue #152: [ROCKETMQ-278] Add clusterlist cmd by specifi...
Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/152 LGTM @zhouxinyu @vongosling ---
[GitHub] incubator-rocketmq issue #153: [ROCKETMQ-272] Fix sync slave timeout when us...
Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/153 @evthoriz may you mock a test for this scenario? ---
[GitHub] incubator-rocketmq issue #154: [Rocketmq-285] file test error when make link
Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/154 @zwillim I want to merge this PR. Could you please change the target branch to apache:develop ? As all the PRs are merged into develop at first. ---
[GitHub] incubator-rocketmq pull request #156: [ROCKETMQ-271] add tools for Analyzing...
Github user dongeforever commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/156#discussion_r139868962
--- Diff:
broker/src/main/java/org/apache/rocketmq/broker/ServerTracerTimeUtil.java ---
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.broker;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import java.util.concurrent.TimeUnit;
+import org.apache.rocketmq.common.ClientTracerTimeUtil;
+import org.apache.rocketmq.common.TracerTime;
+
+public class ServerTracerTimeUtil {
+
+public static Cache tracerTimeCache =
CacheBuilder.newBuilder()
+.maximumSize(1)
+.expireAfterWrite(15, TimeUnit.MINUTES)
+.build();
+
+public static boolean isEnableTracerTime() {
+return ClientTracerTimeUtil.isEnableTracerTime();
+}
+
+public static void addMessageCreateTime(String messageTracerTimeId,
String messageCreateTime) {
+if (messageCreateTime == null || messageCreateTime.length() < 1) {
+return;
+}
+
+TracerTime tracerTime =
tracerTimeCache.getIfPresent(messageTracerTimeId);
+if (tracerTime == null) {
+tracerTime = new TracerTime();
--- End diff --
getIfPresent may have concurrent problem
Maybe getOrDefault is OK
---
[GitHub] incubator-rocketmq pull request #156: [ROCKETMQ-271] add tools for Analyzing...
Github user dongeforever commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/156#discussion_r139869169
--- Diff:
broker/src/main/java/org/apache/rocketmq/broker/mqtrace/TrackerTimeSendMessageHook.java
---
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.broker.mqtrace;
+
+import java.util.Map;
+import org.apache.rocketmq.broker.ServerTracerTimeUtil;
+import org.apache.rocketmq.common.message.MessageConst;
+import org.apache.rocketmq.common.message.MessageDecoder;
+
+public class TrackerTimeSendMessageHook implements SendMessageHook {
+
+@Override
+public String hookName() {
+return "TrackerTimeSendMessageHook";
+}
+
+@Override
+public void sendMessageBefore(SendMessageContext context) {
+// brokerController.getBrokerConfig().isEnableTracerTime()
+String props = context.getMsgProps();
+if (props != null && props.length() > 1) {
+Map properties =
MessageDecoder.string2messageProperties(props);
+String messageTracerTimeId =
properties.get(MessageConst.PROPERTY_UNIQ_CLIENT_MESSAGE_ID_KEYIDX);
+if (properties.containsKey(MessageConst.MESSAGE_CREATE_TIME)) {
+
ServerTracerTimeUtil.addMessageCreateTime(messageTracerTimeId,
properties.get(MessageConst.MESSAGE_CREATE_TIME));
+
ServerTracerTimeUtil.addMessageSendTime(messageTracerTimeId,
properties.get(MessageConst.MESSAGE_SEND_TIME));
+
ServerTracerTimeUtil.addMessageArriveBrokerTime(messageTracerTimeId,
System.currentTimeMillis());
+
ServerTracerTimeUtil.addMessageBeginSaveTime(messageTracerTimeId,
System.currentTimeMillis());
+}
+}
+
+}
+
+@Override
+public void sendMessageAfter(SendMessageContext context) {
+String props = context.getMsgProps();
+if (props != null && props.length() > 1) {
+Map properties =
MessageDecoder.string2messageProperties(props);
+if (properties.containsKey(MessageConst.MESSAGE_CREATE_TIME)) {
+String messageTracerTimeId =
properties.get(MessageConst.PROPERTY_UNIQ_CLIENT_MESSAGE_ID_KEYIDX);
+
ServerTracerTimeUtil.addMessageSaveEndTime(messageTracerTimeId,
System.currentTimeMillis());
+
ServerTracerTimeUtil.addBrokerSendAckTime(messageTracerTimeId,
System.currentTimeMillis());
+
ServerTracerTimeUtil.addBrokerSendAckTime(messageTracerTimeId,
System.currentTimeMillis());
--- End diff --
here two lines are same.
May you want to addReceiveSendAckTime?
---
[GitHub] incubator-rocketmq issue #156: [ROCKETMQ-271] add tools for Analyzing messag...
Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/156 LGTM @zhouxinyu @vongosling ---
[GitHub] incubator-rocketmq issue #165: fix DefaultMessageStoreTest bug: wait more ti...
Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/165 @wenweihu86 By default, the commitlog will flush in real-time. If enable flushCommitLogTimed, then it will flush at the interval. So there is no need to sleep 100 ms, 10 is enough, just for the building time of consume queue. ---
[GitHub] incubator-rocketmq issue #167: Merge pull request #1 from apache/master
Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/167 @warning5 this pr has no files changed. ---
