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

[BUG]fix zkBookieRackAffinityMapping bug to support for bookkeeper dnsResolver #9894

Conversation

hangc0276
Copy link
Contributor

Motivation

When using Region/Rack aware placement policy for pulsar managed ledger, we should also use those policy for bookkeeper and bookkeeper.conf should add the follow configuration. Otherwise, it the placement policy won't work for bookkeeper auto recovery.

ensemblePlacementPolicy=org.apache.bookkeeper.client.RegionAwareEnsemblePlacementPolicy
reppDnsResolverClass=org.apache.pulsar.zookeeper.ZkBookieRackAffinityMapping

However, if we configured change root for Zookeeper in bookkeeper.conf, eg.

zkLedgersRootPath=/test-v1/ledgers
zkServers=localhost:2181

/test-v1 is the change root for zookeeper.

The zkBookieRackAffinityMapping resolver will be invalid, and all resolved result will be null.

The reason is that zkBookieRackAffinityMapping initiate Zookeeper Client according to zkServers configured in bookkeeper.conf, and doesn't take change root into account. Thus it lead to get bookies placement policy from wrong place , which is /bookies and not the correct /test-v1/bookies.

Changes

  1. Check whether configured Zookeeper change root in bookkeeper.conf before initiate Zookeeper Client. If it configured, use the change root path to initiate Zookeeper Client.

…n bookkeeper use zk changeRoot configuration
@hangc0276 hangc0276 self-assigned this Mar 12, 2021
Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

Looks good to me, please add some unit test to cover the changes.

@sijie sijie added this to the 2.8.0 milestone Mar 12, 2021
@sijie
Copy link
Member

sijie commented Mar 12, 2021

/pulsarbot run-failure-checks

1 similar comment
@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

@sijie sijie merged commit 46750d6 into apache:master Mar 17, 2021
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Mar 24, 2021
codelipenghui pushed a commit that referenced this pull request Mar 24, 2021
…sResolver (#9894)

### Motivation
When using Region/Rack aware placement policy for pulsar managed ledger, we should also use those policy for bookkeeper and bookkeeper.conf should add the follow configuration. Otherwise, it the placement policy won't work for bookkeeper auto recovery.
```
ensemblePlacementPolicy=org.apache.bookkeeper.client.RegionAwareEnsemblePlacementPolicy
reppDnsResolverClass=org.apache.pulsar.zookeeper.ZkBookieRackAffinityMapping
```
However, if we configured change root for Zookeeper in bookkeeper.conf, eg.
```
zkLedgersRootPath=/test-v1/ledgers
zkServers=localhost:2181
```
`/test-v1` is the change root for zookeeper.

The `zkBookieRackAffinityMapping` resolver will be invalid, and all resolved result will be `null`.

The reason is that `zkBookieRackAffinityMapping` initiate Zookeeper Client according to `zkServers` configured in `bookkeeper.conf`, and doesn't take change root into account. Thus it lead to get bookies placement policy from wrong place  , which is `/bookies` and not the correct `/test-v1/bookies`.

### Changes
1. Check whether configured Zookeeper change root in `bookkeeper.conf` before initiate Zookeeper Client. If it configured, use the change root path to initiate Zookeeper Client.

(cherry picked from commit 46750d6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.7 Archived: 2.7 is end of life release/2.7.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants