Skip to content
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

[improve][doc] Improve doc for delayed message #20374

Merged
merged 2 commits into from
May 24, 2023

Conversation

coderzc
Copy link
Member

@coderzc coderzc commented May 23, 2023

Motivation

Due to PIP-195 introducing BucketDelayedDeliveryTracker, some document descriptions need to be changed.

Modifications

Improve doc for delayed message tracker.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@coderzc coderzc changed the title Improve delayed message config doc [improve][doc] Improve doc for delayed message May 23, 2023
@github-actions github-actions bot added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label May 23, 2023
@coderzc coderzc force-pushed the improve_delayed_message_doc branch from 5e2c5f1 to ab29df6 Compare May 23, 2023 14:30
@michaeljmarshall
Copy link
Member

/pulsarbot rerun-failure-checks

@@ -582,7 +580,7 @@ delayedDeliveryMaxIndexesPerBucketSnapshotSegment=5000
delayedDeliveryMaxNumBuckets=-1

# Size of the lookahead window to use when detecting if all the messages in the topic
# have a fixed delay.
# have a fixed delay for InMemoryDelayedDeliveryTracker (the default DelayedDeliverTracker).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# have a fixed delay for InMemoryDelayedDeliveryTracker (the default DelayedDeliverTracker).
# have a fixed delay for InMemoryDelayedDeliveryTracker (the default value is DelayedDeliverTracker).

Do you mean this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the default DelayedDeliverTracker is InMemoryDelayedDeliveryTracker

@@ -379,7 +377,8 @@ The delayed message index time step(in seconds) in per bucket snapshot segment,
private int delayedDeliveryMaxNumBuckets = -1;

@FieldContext(category = CATEGORY_SERVER, doc = "Size of the lookahead window to use "
+ "when detecting if all the messages in the topic have a fixed delay. "
+ "when detecting if all the messages in the topic have a fixed delay for "
+ "InMemoryDelayedDeliveryTracker (the default DelayedDeliverTracker). "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@coderzc coderzc closed this May 24, 2023
@coderzc coderzc reopened this May 24, 2023
@Anonymitaet Anonymitaet added this to the 3.1.0 milestone May 24, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #20374 (9c9cdd6) into master (548fd22) will increase coverage by 36.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #20374       +/-   ##
=============================================
+ Coverage     36.89%   72.92%   +36.03%     
- Complexity    12053    31889    +19836     
=============================================
  Files          1687     1864      +177     
  Lines        128826   138387     +9561     
  Branches      14013    15184     +1171     
=============================================
+ Hits          47528   100922    +53394     
+ Misses        75040    29465    -45575     
- Partials       6258     8000     +1742     
Flag Coverage Δ
inttests 24.10% <100.00%> (-0.08%) ⬇️
systests 24.95% <100.00%> (-0.22%) ⬇️
unittests 72.22% <100.00%> (+40.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...mon/policies/data/stats/SubscriptionStatsImpl.java 69.56% <ø> (ø)
...sar/common/policies/data/stats/TopicStatsImpl.java 90.44% <ø> (+36.30%) ⬆️
...org/apache/pulsar/broker/ServiceConfiguration.java 99.37% <100.00%> (+1.35%) ⬆️

... and 1429 files with indirect coverage changes

@tisonkun
Copy link
Member

Merging...

Thank you @coderzc!

@tisonkun tisonkun merged commit 545abfc into apache:master May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Your PR contains doc changes, no matter whether the changes are in markdown or code files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants