-
Notifications
You must be signed in to change notification settings - Fork 532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TUBEMQ-126] Increase the unflushed data bytes control #142
Conversation
…o data persistence.
This modification is not a best practice, this problem needs to be adjusted in conjunction with other points |
@@ -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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should this place be changed to valueof ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the system wants to turn off this check, what should it do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -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(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to adjust the whole assignment order here? Is there any big change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to reuse unflushdatahold field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, it would be nice if the "unflushdatahold" wasn't in use by any latent feature.
; 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if different topics require different flushBySizeThreshold configuration?
tubemq-server/src/main/java/org/apache/tubemq/server/broker/metadata/BrokerDefMetadata.java
Show resolved
Hide resolved
@@ -37,10 +37,8 @@ | |||
private int unflushThreshold = 1000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Co-authored-by: dockerzhang <dockerzhang@tencent.com>
Co-authored-by: dockerzhang <dockerzhang@tencent.com>
Implement the unflushSize as a factor to trigger data flush, and the changes consist of the following 3 parts:
Details: TUBE-126