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

Add preciseTopicPublishRateLimiterEnable to broker.conf #10216

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Apr 13, 2021

Motivation

PR #7078 introduced preciseTopicPublishRateLimiterEnable setting, but this setting is missing from broker.conf. The impact of this is that it is required to use PULSAR_PREFIX_preciseTopicPublishRateLimiterEnable: "true" in values.yaml to enable the setting. It would be more consistent with other settings if preciseTopicPublishRateLimiterEnable: "true" could be used.

Modifications

Add preciseTopicPublishRateLimiterEnable=false to broker.conf.

Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

@lhotari Can you check if we also need to add it to standalone.conf?

@sijie sijie added this to the 2.8.0 milestone Apr 13, 2021
@lhotari
Copy link
Member Author

lhotari commented Apr 14, 2021

Can you check if we also need to add it to standalone.conf?

@sijie I checked now. It would be needed. I'll add it.

There's a lot of other configuration parameters missing from standalone.conf . Would it be necessary to create a separate issue to sync all missing parameters?

@lhotari lhotari force-pushed the lh-add-preciseTopicPublishRateLimiterEnable-to-broker.conf branch from 578ba2f to ccd409b Compare April 15, 2021 18:32
@lhotari
Copy link
Member Author

lhotari commented Apr 16, 2021

/pulsarbot run-failure-checks

@eolivelli eolivelli merged commit 3d6e480 into apache:master Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants