You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/07/27 16:15:53 UTC
[GitHub] [pulsar] AnonHxy opened a new pull request, #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
AnonHxy opened a new pull request, #16824:
URL: https://github.com/apache/pulsar/pull/16824
### Motivation
* There is only a simple word `Default message retention time` on `defaultRetentionTimeInMinutes=0` and `defaultRetentionSizeInMB=0` in broker.conf. I think it is not clear for users because `0` and `-1` are both special numbers but have different meaning here. New users maybe mistaken thought that value of `0` means limit as "infinite" size quota
### Modifications
* Improve `defaultRetentionTimeInMinutes` and `defaultRetentionSizeInMB` doc on *.conf and *.md and `ServiceConfiguration.java`
### Verifying this change
- [x] Make sure that the change passes the CI checks.
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [ ] `doc-not-needed`
(Please explain why)
- [x] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#discussion_r940072485
##########
conf/broker.conf:
##########
@@ -1264,10 +1264,10 @@ replicatorPrefix=pulsar.repl
# due to missing ZooKeeper watch (disable with value 0)
replicationPolicyCheckDurationSeconds=600
-# Default message retention time
+# Default message retention time. Using a value of -1, is disabling message retention time limit
defaultRetentionTimeInMinutes=0
-# Default retention size
+# Default retention size. Using a value of -1, is disabling message retention size limit
Review Comment:
I think it's not correct.
0 means the retention is disabled,
-1 means the data will not be removed by time or size quota?
I think we can change to
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on code in PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#discussion_r940946822
##########
conf/broker.conf:
##########
@@ -1264,10 +1264,10 @@ replicatorPrefix=pulsar.repl
# due to missing ZooKeeper watch (disable with value 0)
replicationPolicyCheckDurationSeconds=600
-# Default message retention time
+# Default message retention time. Using a value of -1, is disabling message retention time limit
defaultRetentionTimeInMinutes=0
-# Default retention size
+# Default retention size. Using a value of -1, is disabling message retention size limit
Review Comment:
Great.
I find this PR https://github.com/apache/pulsar/pull/16572 has already fix. But the description in the doc maybe not clear enought, e. g, :
- https://github.com/apache/pulsar/blob/f02679d83402ac07e06c48570505f343e350a491/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java#L2382
`will not be removed by time or size quota` is better I think
- https://github.com/apache/pulsar/blob/f02679d83402ac07e06c48570505f343e350a491/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java#L2388
`no infinite size quota` should be `infinite size quota`
So I rewrite it as you mentioned above. @codelipenghui . PTAL also @Anonymitaet ~ thanks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on code in PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#discussion_r940905081
##########
conf/broker.conf:
##########
@@ -1264,10 +1264,10 @@ replicatorPrefix=pulsar.repl
# due to missing ZooKeeper watch (disable with value 0)
replicationPolicyCheckDurationSeconds=600
-# Default message retention time
+# Default message retention time. Using a value of -1, is disabling message retention time limit
defaultRetentionTimeInMinutes=0
-# Default retention size
+# Default retention size. Using a value of -1, is disabling message retention size limit
Review Comment:
Great. Have updated, PTAL ~ @codelipenghui @Anonymitaet
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#issuecomment-1225753796
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#discussion_r941963437
##########
conf/broker.conf:
##########
@@ -1309,13 +1309,11 @@ replicatorPrefix=pulsar.repl
replicationPolicyCheckDurationSeconds=600
# Default message retention time.
-# The default value is 0, which means the data is removed after all the subscriptions are consumed.
-# Value less than 0 means messages never expire.
+# Value of 0 means the retention is disabled, -1 means the data will not be removed by time quota.
Review Comment:
```suggestion
# 0 means retention is disabled. -1 means data is not removed by time quota.
```
Write in the simple present tense as much as possible if you are covering facts that were, are, and forever shall be true.
https://docs.google.com/document/d/1lc5j4RtuLIzlEYCBo97AC8-U_3Erzs_lxpkDuseU0n4/edit#bookmark=id.e8uqh1awkcnp
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on code in PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#discussion_r940946822
##########
conf/broker.conf:
##########
@@ -1264,10 +1264,10 @@ replicatorPrefix=pulsar.repl
# due to missing ZooKeeper watch (disable with value 0)
replicationPolicyCheckDurationSeconds=600
-# Default message retention time
+# Default message retention time. Using a value of -1, is disabling message retention time limit
defaultRetentionTimeInMinutes=0
-# Default retention size
+# Default retention size. Using a value of -1, is disabling message retention size limit
Review Comment:
Great.
I find this PR https://github.com/apache/pulsar/pull/16572 has already fix partial of the problem. But the description in the doc maybe not clear enought, e. g, :
- https://github.com/apache/pulsar/blob/f02679d83402ac07e06c48570505f343e350a491/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java#L2382
`will not be removed by time or size quota` is better I think
- https://github.com/apache/pulsar/blob/f02679d83402ac07e06c48570505f343e350a491/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java#L2388
`no infinite size quota` should be `infinite size quota`
So I also rewrite it as what you mentioned above. @codelipenghui . PTAL also @Anonymitaet ~ thanks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yuruguo commented on pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
yuruguo commented on PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#issuecomment-1225698036
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on code in PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#discussion_r942020066
##########
conf/broker.conf:
##########
@@ -1309,13 +1309,11 @@ replicatorPrefix=pulsar.repl
replicationPolicyCheckDurationSeconds=600
# Default message retention time.
-# The default value is 0, which means the data is removed after all the subscriptions are consumed.
-# Value less than 0 means messages never expire.
+# Value of 0 means the retention is disabled, -1 means the data will not be removed by time quota.
Review Comment:
Updated. PTAL @Anonymitaet
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yuruguo merged pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
yuruguo merged PR #16824:
URL: https://github.com/apache/pulsar/pull/16824
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on code in PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#discussion_r940905081
##########
conf/broker.conf:
##########
@@ -1264,10 +1264,10 @@ replicatorPrefix=pulsar.repl
# due to missing ZooKeeper watch (disable with value 0)
replicationPolicyCheckDurationSeconds=600
-# Default message retention time
+# Default message retention time. Using a value of -1, is disabling message retention time limit
defaultRetentionTimeInMinutes=0
-# Default retention size
+# Default retention size. Using a value of -1, is disabling message retention size limit
Review Comment:
Great. Have updated, PTAL ~ @codelipenghui @Anonymitaet
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#issuecomment-1208945527
I find this PR [Default message retention time](https://github.com/apache/pulsar/pull/16572) has already improved partial of the document. So what I need do in this PR is complete the rest
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on code in PR #16824:
URL: https://github.com/apache/pulsar/pull/16824#discussion_r940905081
##########
conf/broker.conf:
##########
@@ -1264,10 +1264,10 @@ replicatorPrefix=pulsar.repl
# due to missing ZooKeeper watch (disable with value 0)
replicationPolicyCheckDurationSeconds=600
-# Default message retention time
+# Default message retention time. Using a value of -1, is disabling message retention time limit
defaultRetentionTimeInMinutes=0
-# Default retention size
+# Default retention size. Using a value of -1, is disabling message retention size limit
Review Comment:
Great
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org