Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[fix][doc] Describe approximate behavior of time-based quotas #412

Conversation

teabot
Copy link

@teabot teabot commented Feb 15, 2023

Time-based quotas are applied approximately. Yet the documentation supposes they are strict. It would be useful to set proper user expectations.

Documentation

image

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

Link to config

Older versions
@github-actions github-actions bot added the doc Improvements or additions to documentation label Feb 15, 2023
@tisonkun tisonkun self-requested a review February 16, 2023 23:33
@tisonkun tisonkun requested a review from Technoboy- February 16, 2023 23:34
@@ -214,6 +214,8 @@ Backlog quotas are handled at the namespace level. They can be managed via:

You can set a size and/or time threshold and backlog retention policy for all of the topics in a [namespace](reference-terminology.md#namespace) by specifying the namespace, a size limit and/or a time limit in second, and a policy by name.

Note that by default, time-based backlogs are enforced periodically using an approximate method. This avoids a potentially costly scan of the backlog each time a message is produced. However, it does mean that in some cases you may observe a lack of strict enforcement. To tune this behavior you should consider using the [`backlogQuotaCheckIntervalInSeconds`][backlogquotacheckintervalinseconds] and [`preciseTimeBasedBacklogQuotaCheck`][precisetimebasedbacklogquotacheck] broker options.
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
Note that by default, time-based backlogs are enforced periodically using an approximate method. This avoids a potentially costly scan of the backlog each time a message is produced. However, it does mean that in some cases you may observe a lack of strict enforcement. To tune this behavior you should consider using the [`backlogQuotaCheckIntervalInSeconds`][backlogquotacheckintervalinseconds] and [`preciseTimeBasedBacklogQuotaCheck`][precisetimebasedbacklogquotacheck] broker options.
::: note
By default, time-based backlogs are enforced periodically using an approximate method. This avoids a potentially costly scan of the backlog each time a message is produced. However, it does mean that in some cases you may observe a lack of strict enforcement. To tune this behavior, you should consider using the [`backlogQuotaCheckIntervalInSeconds`][backlogquotacheckintervalinseconds] and [`preciseTimeBasedBacklogQuotaCheck`][precisetimebasedbacklogquotacheck] broker options.
:::

Copy link
Member

Choose a reason for hiding this comment

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

This suggestions apply to other md files as well. Can you update them all?

Copy link
Author

Choose a reason for hiding this comment

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

sure

Co-authored-by: Anonymitaet <50226895+Anonymitaet@users.noreply.github.com>
@@ -214,6 +214,12 @@ Backlog quotas are handled at the namespace level. They can be managed via:

You can set a size and/or time threshold and backlog retention policy for all of the topics in a [namespace](reference-terminology.md#namespace) by specifying the namespace, a size limit and/or a time limit in second, and a policy by name.

::: note

By default, time-based backlogs are enforced periodically using an approximate method. This avoids a potentially costly scan of the backlog each time a message is produced. However, it does mean that in some cases you may observe a lack of strict enforcement. To tune this behavior, you should consider using the [`backlogQuotaCheckIntervalInSeconds`][backlogquotacheckintervalinseconds] and [`preciseTimeBasedBacklogQuotaCheck`][precisetimebasedbacklogquotacheck] broker options.
Copy link
Member

Choose a reason for hiding this comment

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

what does this mean?

[xxx][xxx]

image

Do you want to highlight the para? You can use xxx instead.

Copy link
Author

@teabot teabot Mar 6, 2023

Choose a reason for hiding this comment

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

Comment on lines +483 to +484
[backlogquotacheckintervalinseconds]: https://pulsar.apache.org/reference/#/next/config/reference-configuration-broker?id=backlogquotacheckintervalinseconds
[precisetimebasedbacklogquotacheck]: https://pulsar.apache.org/reference/#/next/config/reference-configuration-broker?id=precisetimebasedbacklogquotacheck
Copy link
Member

Choose a reason for hiding this comment

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

  1. What's the context of these two paras? Do you intend mean this?
Suggested change
[backlogquotacheckintervalinseconds]: https://pulsar.apache.org/reference/#/next/config/reference-configuration-broker?id=backlogquotacheckintervalinseconds
[precisetimebasedbacklogquotacheck]: https://pulsar.apache.org/reference/#/next/config/reference-configuration-broker?id=precisetimebasedbacklogquotacheck
To configure checks, you can use `backlogquotacheckintervalinseconds` and
`precisetimebasedbacklogquotacheck` parameters. Details see **Configuration > Plusar > Broker** on [Pulsar Reference Site](https://pulsar.apache.org/reference).
  1. Suggest using general link (https://pulsar.apache.org/reference) instead of specific links to reduce maintenance costs and potential errors. We've applied this strategy across all docs.

Copy link
Author

Choose a reason for hiding this comment

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

I disagree with this practice. It may be easier for authors, but it's really difficult for readers to find what they need. The target page has 100s of elements, is nested, is not searchable, and is not sorted.

I see in your suggestion that you still provide context to the reader "see Configuration > Plusar > Broker" — doesn't this too suffer from the same issues of maintenance and errors, while also being less useful by not being encoded in the link?

Copy link
Member

@Anonymitaet Anonymitaet Mar 8, 2023

Choose a reason for hiding this comment

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

This comment was left two weeks ago. The reason for my suggestion (as a temporary workaround) at that time was:

For example, if we want to publish a new next code release (2.12.0) with docs, the whole 2.12.x doc set is copied from master. If we use xxx/next/xxx in links, then the links in the old doc set will always point to the latest Reference site, which is incorrect.

< < < < < < < < < < < < < < < <

But now you can write it as [precisetimebasedbacklogquotacheck]: https://pulsar.apache.org/reference/#/@pulsar:version_origin@/config/reference-configuration-broker?id=precisetimebasedbacklogquotacheck since #456 was merged yesterday.

@@ -214,6 +214,12 @@ Backlog quotas are handled at the namespace level. They can be managed via:

You can set a size and/or time threshold and backlog retention policy for all of the topics in a [namespace](reference-terminology.md#namespace) by specifying the namespace, a size limit and/or a time limit in second, and a policy by name.

::: note
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
::: note
:::note

@tisonkun
Copy link
Member

Code conflict and stale for a while. I'd close this PR and feel free to recreate a PR based on the latest master and ping me for review.

@tisonkun tisonkun closed this May 25, 2023
@tisonkun
Copy link
Member

Note that it doesn't mean this PR is rejected - it's good to have. Just a notification to roll up the stale PR.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
doc Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants