You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tubemq.apache.org by GitBox <gi...@apache.org> on 2020/06/09 16:33:00 UTC
[GitHub] [incubator-tubemq] hystericalhell opened a new pull request #142: [TUBEMQ-126] Increase the unflushed data bytes control
hystericalhell opened a new pull request #142:
URL: https://github.com/apache/incubator-tubemq/pull/142
Implement the unflushSize as a factor to trigger data flush, and the changes consist of the following 3 parts:
1. MsgFileStore: Make use of curUnflushSize and UnflushSizeThreshold settings to trigger;
2. Metadata: Support new config on UnflushSizeThreshold;
3. TestSuite: Add unit test cases with newly introduced UnflushSizeThreshold.
Details: [TUBE-126](https://issues.apache.org/jira/browse/TUBEMQ-126)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tubemq] hystericalhell commented on a change in pull request #142: [TUBEMQ-126] Increase the unflushed data bytes control
Posted by GitBox <gi...@apache.org>.
hystericalhell commented on a change in pull request #142:
URL: https://github.com/apache/incubator-tubemq/pull/142#discussion_r443503970
##########
File path: tubemq-server/src/main/java/org/apache/tubemq/server/broker/msgstore/MessageStore.java
##########
@@ -559,13 +566,13 @@ private long parseDeletePolicy(String delPolicy) {
String validValStr = tmpStrs[1];
try {
if (validValStr.endsWith("m")) {
- return Long.parseLong(validValStr.substring(0, validValStr.length() - 1)) * 60000;
+ return Long.valueOf(validValStr.substring(0, validValStr.length() - 1)) * 60000;
Review comment:
Interestingly I didn't mean to change it either, I will revert them, maybe I clicked something in IDE which did something automatically in background.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tubemq] gosonzhang commented on pull request #142: [TUBEMQ-126] Increase the unflushed data bytes control
Posted by GitBox <gi...@apache.org>.
gosonzhang commented on pull request #142:
URL: https://github.com/apache/incubator-tubemq/pull/142#issuecomment-643930917
This modification is not a best practice, this problem needs to be adjusted in conjunction with other points
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tubemq] hystericalhell commented on a change in pull request #142: [TUBEMQ-126] Increase the unflushed data bytes control
Posted by GitBox <gi...@apache.org>.
hystericalhell commented on a change in pull request #142:
URL: https://github.com/apache/incubator-tubemq/pull/142#discussion_r443505255
##########
File path: tubemq-server/src/main/java/org/apache/tubemq/server/broker/metadata/TopicMetadata.java
##########
@@ -95,57 +97,63 @@ public TopicMetadata(final BrokerDefMetadata brokerDefMetadata, String topicMeta
this.unflushInterval = Integer.parseInt(topicConfInfoArr[5]);
}
if (TStringUtils.isBlank(topicConfInfoArr[6])) {
- this.deleteWhen = brokerDefMetadata.getDeleteWhen();
+ this.unflushSizeThreshold = brokerDefMetadata.getUnflushSizeThreshold();
Review comment:
Easier for read, since it would be hard to realize the sequence if it evaluates topicConfInfoArr[7] and then a topicConfInfoArr[14] follows, people would feel weird about what's happening to 8-13 and guess whether they are no-in-use.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tubemq] hystericalhell closed pull request #142: [TUBEMQ-126] Increase the unflushed data bytes control
Posted by GitBox <gi...@apache.org>.
hystericalhell closed pull request #142:
URL: https://github.com/apache/incubator-tubemq/pull/142
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tubemq] hystericalhell commented on a change in pull request #142: [TUBEMQ-126] Increase the unflushed data bytes control
Posted by GitBox <gi...@apache.org>.
hystericalhell commented on a change in pull request #142:
URL: https://github.com/apache/incubator-tubemq/pull/142#discussion_r443505874
##########
File path: tubemq-server/src/main/java/org/apache/tubemq/server/broker/metadata/TopicMetadata.java
##########
@@ -37,6 +37,8 @@
private int unflushThreshold = 1000;
// data will be flushed to disk when unflushed message count exceed this.
private int unflushInterval = 10000;
+ // data will be flushed to disk when unflushed message accumulated size exceed threshold, default to 64MB.
Review comment:
I think the best solution is to setup a config in broker.ini, and disable its control if it's set to 0 or below.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tubemq] hystericalhell commented on a change in pull request #142: [TUBEMQ-126] Increase the unflushed data bytes control
Posted by GitBox <gi...@apache.org>.
hystericalhell commented on a change in pull request #142:
URL: https://github.com/apache/incubator-tubemq/pull/142#discussion_r443506367
##########
File path: tubemq-server/src/main/java/org/apache/tubemq/server/broker/metadata/BrokerDefMetadata.java
##########
@@ -36,6 +36,8 @@
private int unflushDataHold = 10000;
// data will be flushed to disk when elapse unflushInterval milliseconds since last flush operation.
private int unflushInterval = 10000;
+ // data will be flushed to disk when unflushed message accumulated size exceed threshold, default to 64MB.
+ private long unflushSizeThreshold = 1 << 26;
Review comment:
Sure, it would be nice if the "unflushdatahold" wasn't in use by any latent feature.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tubemq] gosonzhang commented on a change in pull request #142: [TUBEMQ-126] Increase the unflushed data bytes control
Posted by GitBox <gi...@apache.org>.
gosonzhang commented on a change in pull request #142:
URL: https://github.com/apache/incubator-tubemq/pull/142#discussion_r443180243
##########
File path: tubemq-server/src/main/java/org/apache/tubemq/server/broker/msgstore/MessageStore.java
##########
@@ -559,13 +566,13 @@ private long parseDeletePolicy(String delPolicy) {
String validValStr = tmpStrs[1];
try {
if (validValStr.endsWith("m")) {
- return Long.parseLong(validValStr.substring(0, validValStr.length() - 1)) * 60000;
+ return Long.valueOf(validValStr.substring(0, validValStr.length() - 1)) * 60000;
Review comment:
Why should this place be changed to valueof ？
##########
File path: tubemq-server/src/main/java/org/apache/tubemq/server/broker/metadata/TopicMetadata.java
##########
@@ -37,6 +37,8 @@
private int unflushThreshold = 1000;
// data will be flushed to disk when unflushed message count exceed this.
private int unflushInterval = 10000;
+ // data will be flushed to disk when unflushed message accumulated size exceed threshold, default to 64MB.
Review comment:
If the system wants to turn off this check, what should it do?
##########
File path: tubemq-server/src/main/java/org/apache/tubemq/server/broker/metadata/TopicMetadata.java
##########
@@ -95,57 +97,63 @@ public TopicMetadata(final BrokerDefMetadata brokerDefMetadata, String topicMeta
this.unflushInterval = Integer.parseInt(topicConfInfoArr[5]);
}
if (TStringUtils.isBlank(topicConfInfoArr[6])) {
- this.deleteWhen = brokerDefMetadata.getDeleteWhen();
+ this.unflushSizeThreshold = brokerDefMetadata.getUnflushSizeThreshold();
Review comment:
Why do we need to adjust the whole assignment order here? Is there any big change?
##########
File path: tubemq-server/src/main/java/org/apache/tubemq/server/broker/metadata/BrokerDefMetadata.java
##########
@@ -36,6 +36,8 @@
private int unflushDataHold = 10000;
// data will be flushed to disk when elapse unflushInterval milliseconds since last flush operation.
private int unflushInterval = 10000;
+ // data will be flushed to disk when unflushed message accumulated size exceed threshold, default to 64MB.
+ private long unflushSizeThreshold = 1 << 26;
Review comment:
It's better to reuse unflushdatahold field.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tubemq] gosonzhang commented on a change in pull request #142: [TUBEMQ-126] Increase the unflushed data bytes control
Posted by GitBox <gi...@apache.org>.
gosonzhang commented on a change in pull request #142:
URL: https://github.com/apache/incubator-tubemq/pull/142#discussion_r446744239
##########
File path: conf/broker.ini
##########
@@ -38,7 +38,10 @@ transferSize= 524288
loadMessageStoresInParallel=true
; timeout of consumer heartbeat, optional; default is 30s
consumerRegTimeoutMs=35000
-
+; whether to enable flush by cached data size threshold
+enableFlushBySize=true
+; the data size threshold in Bytes, default is 64M, if enableFlushBySize=true, or ignored
+flushBySizeThreshold=67108864
Review comment:
What if different topics require different flushBySizeThreshold configuration?
##########
File path: tubemq-server/src/main/java/org/apache/tubemq/server/broker/metadata/BrokerDefMetadata.java
##########
@@ -33,11 +33,9 @@
// data will be flushed to disk when unflushed message count exceed this.
private int unflushThreshold = 1000;
@Deprecated
- private int unflushDataHold = 10000;
+ private long unflushDataHold = 10000;
Review comment:
int is enough
##########
File path: tubemq-server/src/main/java/org/apache/tubemq/server/broker/metadata/TopicMetadata.java
##########
@@ -37,10 +37,8 @@
private int unflushThreshold = 1000;
Review comment:
Can you revert the history changes, then rebase codes, and then submit the final changes again? The historical changes affect the comparison of the changes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org