-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[DO NOT MERGE][Doc] Rename two parameters about metadatastore (ZK free) and add a comprehensive list for deprecated parameters of broker #13758
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
Conversation
1. Add a comprehensive list of deprecated parameters under Broker configurations. 2. Change the following two parameter names in .md files except “puslar-perf managed-ledger zookeeperServers”. * Replace “zookeeperServers” with “metadataStoreUrl”. * Replace “configurationStoreServers” with “configurationMetadataStoreUrl” 3. Add “my-” prefix to ZK node name and port number 2181.
|
@momo-jun:Thanks for your contribution. For this PR, do we need to update docs? |
|
@momo-jun:Thanks for providing doc info! |
Anonymitaet
left a comment
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.
@momo-jun thanks for your contribution!
-
can you modify the PR title? It should be the summary of your PR changes, thanks
-
can you @ related engineers to review this PR?
| |tlsEnabled| Use `webServicePortTls` and `brokerServicePortTls` instead. |false| | ||
| |replicationTlsEnabled| Enable TLS when talking with other clusters to replicate messages |false| | ||
| |subscriptionKeySharedEnable| Whether to enable the Key_Shared subscription.|true| | ||
| |zookeeperServers| Zookeeper quorum connection string. Use `metadataStoreUrl` instead. || |
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.
| |zookeeperServers| Zookeeper quorum connection string. Use `metadataStoreUrl` instead. || | |
| |zookeeperServers| Zookeeper quorum connection string. Use `metadataStoreUrl` instead. |N/A| |
If no default value, does it make sense to add "N/A" to tell users it does not have a default value rather than empty (tells nothing)?
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.
Thanks Yu. Updated.
| |replicationTlsEnabled| Enable TLS when talking with other clusters to replicate messages |false| | ||
| |subscriptionKeySharedEnable| Whether to enable the Key_Shared subscription.|true| | ||
| |zookeeperServers| Zookeeper quorum connection string. Use `metadataStoreUrl` instead. || | ||
| |configurationStoreServers| Configuration store connection string (as a comma-separated list). Use `configurationMetadataStoreUrl` instead. || |
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.
same comment
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.
Thanks Yu. Updated.
Thanks Yu. Will do that. |
site2/docs/administration-proxy.md
Outdated
| zookeeperServers=zk-0,zk-1,zk-2 | ||
| configurationStoreServers=zk-0:2184,zk-remote:2184 | ||
| metadataStoreUrl=my-zk-0:2181,my-zk-1:2181,my-zk-2:2181 | ||
| configurationMetadataStoreUrl=my-zk-0:2184,my-zk-remote:2184 |
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.
No need to update this file.
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.
Once #13777 will be merged to master, you can ignore this review.
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.
Once #13777 will be merged to master, you can ignore this review.
Agree. Thanks for pointing this out.
site2/docs/functions-runtime.md
Outdated
| ```Yaml | ||
| clientAuthenticationPlugin: org.apache.pulsar.client.impl.auth.AuthenticationToken | ||
| clientAuthenticationParameters: file:///etc/pulsar/token/admin-token.txt | ||
| configurationStoreServers: zookeeper-cluster:2181 # auth requires a connection to zookeeper |
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.
No need to update this file.
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.
Once #13782 will be merged to master, you can ignore this review.
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.
Once #13782 will be merged to master, you can ignore this review.
Agree. Thanks for pointing this out.
site2/docs/functions-worker.md
Outdated
| authorizationEnabled: true | ||
| authorizationProvider: org.apache.pulsar.broker.authorization.PulsarAuthorizationProvider | ||
| configurationStoreServers: <configuration-store-servers> | ||
| configurationMetadataStoreUrl: <configuration-store-servers> |
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.
No need to update this file.
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.
Once #13782 will be merged to master, you can ignore this review.
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.
Once #13782 will be merged to master, you can ignore this review.
Agree. Thanks for pointing this out.
| The [Pulsar proxy](concepts-architecture-overview.md#pulsar-proxy) can be configured in the `conf/proxy.conf` file. | ||
|
|
||
|
|
||
| |Name|Description|Default| |
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.
This section does not need to be updated.
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.
Once #13777 will be merged to master, you can ignore this review.
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.
Once #13777 will be merged to master, you can ignore this review.
Agree. Thanks for pointing this out.
|
@momo-jun can you resolve the conflicts in this PR? |
@Anonymitaet I've resolved the conflicts with PR#14081. |
Co-authored-by: Zike Yang <zkyang@streamnative.io>
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.
Add a description to explain the replacement for subscriptionKeySharedEnable as follows.
Use subscriptionTypesEnabled instead.
…omprehensive list for deprecated parameters of broker (apache#13758)
As per #13077, the following changes were implemented in the docs:
Add a comprehensive list of deprecated parameters under Broker configurations.

Change the following two parameter names in .md files except “puslar-perf managed-ledger zookeeperServers”.
zookeeperServerswithmetadataStoreUrl.configurationStoreServerswithconfigurationMetadataStoreUrl.Add a prefix

my-to ZK node name as well as client port number 2181.doc