Skip to content

[DO NOT MERGE][Doc] Rename two parameters about metadatastore (ZK free) and add a comprehensive list for deprecated parameters of broker - #13758

Merged
Anonymitaet merged 6 commits into
apache:masterfrom
momo-jun:2022-1-14
Feb 8, 2022

Conversation

@momo-jun

@momo-jun momo-jun commented Jan 14, 2022

Copy link
Copy Markdown
Contributor

As per #13077, the following changes were implemented in the docs:

  1. Add a comprehensive list of deprecated parameters under Broker configurations.
    image

  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 a prefix my- to ZK node name as well as client port number 2181.
    image

  • doc

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.
@github-actions

Copy link
Copy Markdown

@momo-jun:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@github-actions

Copy link
Copy Markdown

@momo-jun:Thanks for providing doc info!

@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 Jan 14, 2022

@Anonymitaet Anonymitaet left a comment

Copy link
Copy Markdown
Member

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!

  1. can you modify the PR title? It should be the summary of your PR changes, thanks

  2. can you @ related engineers to review this PR?

Comment thread site2/docs/reference-configuration.md Outdated
|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. ||

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
|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)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks Yu. Updated.

Comment thread site2/docs/reference-configuration.md Outdated
|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. ||

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks Yu. Updated.

@momo-jun momo-jun changed the title Doc update for PR#13077 Rename two parameters about ZK and add a comprehensive list for deprecated parameters of broker Jan 17, 2022
@momo-jun

Copy link
Copy Markdown
Contributor Author

@momo-jun thanks for your contribution!

  1. can you modify the PR title? It should be the summary of your PR changes, thanks
  2. can you @ related engineers to review this PR?

Thanks Yu. Will do that.

@momo-jun

Copy link
Copy Markdown
Contributor Author

Hi @merlimat @nodece, can you please help review this PR for doc updates? Thanks a lot.

@momo-jun momo-jun changed the title Rename two parameters about ZK and add a comprehensive list for deprecated parameters of broker Rename two parameters about metadatastore (ZK free) and add a comprehensive list for deprecated parameters of broker Jan 18, 2022
@momo-jun momo-jun changed the title Rename two parameters about metadatastore (ZK free) and add a comprehensive list for deprecated parameters of broker [Doc] Rename two parameters about metadatastore (ZK free) and add a comprehensive list for deprecated parameters of broker Jan 18, 2022
Comment thread 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

@nodece nodece Jan 20, 2022

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor Author

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.

Comment thread 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

@nodece nodece Jan 20, 2022

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor Author

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.

Comment thread site2/docs/functions-worker.md Outdated
authorizationEnabled: true
authorizationProvider: org.apache.pulsar.broker.authorization.PulsarAuthorizationProvider
configurationStoreServers: <configuration-store-servers>
configurationMetadataStoreUrl: <configuration-store-servers>

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor Author

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.

@@ -695,8 +707,8 @@ The [Pulsar proxy](concepts-architecture-overview.md#pulsar-proxy) can be config
|Name|Description|Default|

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor Author

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.

@Anonymitaet Anonymitaet added this to the 2.10.0 milestone Jan 24, 2022
@Anonymitaet Anonymitaet changed the title [Doc] Rename two parameters about metadatastore (ZK free) and add a comprehensive list for deprecated parameters of broker [DO NOT MERGE][Doc] Rename two parameters about metadatastore (ZK free) and add a comprehensive list for deprecated parameters of broker Jan 24, 2022
@Anonymitaet

Copy link
Copy Markdown
Member

@momo-jun can you resolve the conflicts in this PR?

@momo-jun

momo-jun commented Feb 7, 2022

Copy link
Copy Markdown
Contributor Author

@momo-jun can you resolve the conflicts in this PR?

@Anonymitaet I've resolved the conflicts with PR#14081.
Hi @RobertIndie, can you pls help review the modifications? Thanks.

Comment thread site2/docs/reference-configuration.md Outdated
Comment thread site2/docs/reference-configuration.md Outdated
Co-authored-by: Zike Yang <zkyang@streamnative.io>

@momo-jun momo-jun left a comment

Copy link
Copy Markdown
Contributor Author

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.

Comment thread site2/docs/reference-configuration.md Outdated
@Anonymitaet
Anonymitaet merged commit 32700c9 into apache:master Feb 8, 2022
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
…omprehensive list for deprecated parameters of broker (apache#13758)
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