[Branch-2.10] [fix] [broker] Fix bookeeper packages npe #17291
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/pulsarbot run-failure-checks |
github-actions
bot
added
doc-label-missing
and removed
doc-not-needed
|
@horizonzy Please provide a correct documentation label for your PR. |
github-actions
bot
added
doc-not-needed
Technoboy-
approved these changes
Aug 28, 2022
nicoloboschi
pushed a commit
to datastax/pulsar
that referenced
this pull request
Aug 30, 2022
(cherry picked from commit a7b2772)
BewareMyPower
added a commit
that referenced
this pull request
Sep 13, 2022
…)" This reverts commit a7b2772 to fix cpp tests.
|
@horizonzy I reverted this PR in branch-2.10. It looks like this PR brings an incompatibility issue. |
|
@horizonzy I cherry-picked this PR again. See 1df5734. It seems like the bug can be fixed by cherry-picking #16969. |
|
Fine. |
1 task
michaeljmarshall
pushed a commit
that referenced
this pull request
Sep 16, 2022
…tible across different versions (#17690) * [fix][functions] Ensure InternalConfigurationData data model is compatible across different versions * style * fix the other way ### Motivation After #14384, the broker and the client expects that the `InternalConfigurationData` contains `metadataStoreUrl` and `configurationMetadataStoreUrl` fields. However the broker is no more compatible with old clients. #14384 is landed to branch-2.11 and [2.10.2](#17291) Example scenario: - broker on 2.10.1 - function worker on 2.10.1 1. upgrade fn worker to 2.11.0 or 2.10.2 2. the fn worker starts and download the internal config from the broker 3. broker serves a json with old fields (`zookeeperServers` and `configurationStoreServers`) 4. fn worker reads the json and convert it to a `InternalConfigurationData` instance. It expects to see the fields filled `metadataStoreUrl` and `configurationMetadataStoreUrl` but they aren't 5. NPE on fn worker ``` 2022-09-15T17:42:16,072+0000 [main] INFO org.apache.pulsar.functions.worker.PulsarWorkerService - Initializing Pulsar Functions namespace... 2022-09-15T17:42:16,192+0000 [main] ERROR org.apache.pulsar.functions.worker.FunctionWorkerStarter - Encountered error in function worker. java.lang.NullPointerException: null at org.apache.pulsar.metadata.impl.MetadataStoreFactoryImpl.removeIdentifierFromMetadataURL(MetadataStoreFactoryImpl.java:73) at org.apache.pulsar.functions.worker.WorkerUtils.initializeDlogNamespace(WorkerUtils.java:188) at org.apache.pulsar.functions.worker.PulsarWorkerService.initializeStandaloneWorkerService(PulsarWorkerService.java:281) at org.apache.pulsar.functions.worker.PulsarWorkerService.initAsStandalone(PulsarWorkerService.java:208) at org.apache.pulsar.functions.worker.Worker.start(Worker.java:54) at org.apache.pulsar.functions.worker.FunctionWorkerStarter.main(FunctionWorkerStarter.java:76) ``` Additionaly there's the same issue if we upgrade the broker before the fn worker: 1. the broker gets the upgrade. won't serve `zookeeperServers` field 2. fn worker restarts for some reasons. 3. fn worker gets the internal config and look for `zookeeperServers` field which is empty in the json 4. NPE ### Modifications * Restore old fields in `InternalConfigurationData` and add fallback the old values in the new fields getters * Added unit test - [x] `doc-not-needed`
michaeljmarshall
pushed a commit
that referenced
this pull request
Sep 16, 2022
…tible across different versions (#17690) * [fix][functions] Ensure InternalConfigurationData data model is compatible across different versions * style * fix the other way After #14384, the broker and the client expects that the `InternalConfigurationData` contains `metadataStoreUrl` and `configurationMetadataStoreUrl` fields. However the broker is no more compatible with old clients. #14384 is landed to branch-2.11 and [2.10.2](#17291) Example scenario: - broker on 2.10.1 - function worker on 2.10.1 1. upgrade fn worker to 2.11.0 or 2.10.2 2. the fn worker starts and download the internal config from the broker 3. broker serves a json with old fields (`zookeeperServers` and `configurationStoreServers`) 4. fn worker reads the json and convert it to a `InternalConfigurationData` instance. It expects to see the fields filled `metadataStoreUrl` and `configurationMetadataStoreUrl` but they aren't 5. NPE on fn worker ``` 2022-09-15T17:42:16,072+0000 [main] INFO org.apache.pulsar.functions.worker.PulsarWorkerService - Initializing Pulsar Functions namespace... 2022-09-15T17:42:16,192+0000 [main] ERROR org.apache.pulsar.functions.worker.FunctionWorkerStarter - Encountered error in function worker. java.lang.NullPointerException: null at org.apache.pulsar.metadata.impl.MetadataStoreFactoryImpl.removeIdentifierFromMetadataURL(MetadataStoreFactoryImpl.java:73) at org.apache.pulsar.functions.worker.WorkerUtils.initializeDlogNamespace(WorkerUtils.java:188) at org.apache.pulsar.functions.worker.PulsarWorkerService.initializeStandaloneWorkerService(PulsarWorkerService.java:281) at org.apache.pulsar.functions.worker.PulsarWorkerService.initAsStandalone(PulsarWorkerService.java:208) at org.apache.pulsar.functions.worker.Worker.start(Worker.java:54) at org.apache.pulsar.functions.worker.FunctionWorkerStarter.main(FunctionWorkerStarter.java:76) ``` Additionaly there's the same issue if we upgrade the broker before the fn worker: 1. the broker gets the upgrade. won't serve `zookeeperServers` field 2. fn worker restarts for some reasons. 3. fn worker gets the internal config and look for `zookeeperServers` field which is empty in the json 4. NPE * Restore old fields in `InternalConfigurationData` and add fallback the old values in the new fields getters * Added unit test - [x] `doc-not-needed` (cherry picked from commit 1a34b87)
michaeljmarshall
pushed a commit
that referenced
this pull request
Sep 16, 2022
…tible across different versions (#17690) * [fix][functions] Ensure InternalConfigurationData data model is compatible across different versions * style * fix the other way ### Motivation After #14384, the broker and the client expects that the `InternalConfigurationData` contains `metadataStoreUrl` and `configurationMetadataStoreUrl` fields. However the broker is no more compatible with old clients. #14384 is landed to branch-2.11 and [2.10.2](#17291) Example scenario: - broker on 2.10.1 - function worker on 2.10.1 1. upgrade fn worker to 2.11.0 or 2.10.2 2. the fn worker starts and download the internal config from the broker 3. broker serves a json with old fields (`zookeeperServers` and `configurationStoreServers`) 4. fn worker reads the json and convert it to a `InternalConfigurationData` instance. It expects to see the fields filled `metadataStoreUrl` and `configurationMetadataStoreUrl` but they aren't 5. NPE on fn worker ``` 2022-09-15T17:42:16,072+0000 [main] INFO org.apache.pulsar.functions.worker.PulsarWorkerService - Initializing Pulsar Functions namespace... 2022-09-15T17:42:16,192+0000 [main] ERROR org.apache.pulsar.functions.worker.FunctionWorkerStarter - Encountered error in function worker. java.lang.NullPointerException: null at org.apache.pulsar.metadata.impl.MetadataStoreFactoryImpl.removeIdentifierFromMetadataURL(MetadataStoreFactoryImpl.java:73) at org.apache.pulsar.functions.worker.WorkerUtils.initializeDlogNamespace(WorkerUtils.java:188) at org.apache.pulsar.functions.worker.PulsarWorkerService.initializeStandaloneWorkerService(PulsarWorkerService.java:281) at org.apache.pulsar.functions.worker.PulsarWorkerService.initAsStandalone(PulsarWorkerService.java:208) at org.apache.pulsar.functions.worker.Worker.start(Worker.java:54) at org.apache.pulsar.functions.worker.FunctionWorkerStarter.main(FunctionWorkerStarter.java:76) ``` Additionaly there's the same issue if we upgrade the broker before the fn worker: 1. the broker gets the upgrade. won't serve `zookeeperServers` field 2. fn worker restarts for some reasons. 3. fn worker gets the internal config and look for `zookeeperServers` field which is empty in the json 4. NPE ### Modifications * Restore old fields in `InternalConfigurationData` and add fallback the old values in the new fields getters * Added unit test - [x] `doc-not-needed` (cherry picked from commit 1a34b87)
nicoloboschi
added a commit
to datastax/pulsar
that referenced
this pull request
Sep 20, 2022
…tible across different versions (apache#17690) * [fix][functions] Ensure InternalConfigurationData data model is compatible across different versions * style * fix the other way After apache#14384, the broker and the client expects that the `InternalConfigurationData` contains `metadataStoreUrl` and `configurationMetadataStoreUrl` fields. However the broker is no more compatible with old clients. apache#14384 is landed to branch-2.11 and [2.10.2](apache#17291) Example scenario: - broker on 2.10.1 - function worker on 2.10.1 1. upgrade fn worker to 2.11.0 or 2.10.2 2. the fn worker starts and download the internal config from the broker 3. broker serves a json with old fields (`zookeeperServers` and `configurationStoreServers`) 4. fn worker reads the json and convert it to a `InternalConfigurationData` instance. It expects to see the fields filled `metadataStoreUrl` and `configurationMetadataStoreUrl` but they aren't 5. NPE on fn worker ``` 2022-09-15T17:42:16,072+0000 [main] INFO org.apache.pulsar.functions.worker.PulsarWorkerService - Initializing Pulsar Functions namespace... 2022-09-15T17:42:16,192+0000 [main] ERROR org.apache.pulsar.functions.worker.FunctionWorkerStarter - Encountered error in function worker. java.lang.NullPointerException: null at org.apache.pulsar.metadata.impl.MetadataStoreFactoryImpl.removeIdentifierFromMetadataURL(MetadataStoreFactoryImpl.java:73) at org.apache.pulsar.functions.worker.WorkerUtils.initializeDlogNamespace(WorkerUtils.java:188) at org.apache.pulsar.functions.worker.PulsarWorkerService.initializeStandaloneWorkerService(PulsarWorkerService.java:281) at org.apache.pulsar.functions.worker.PulsarWorkerService.initAsStandalone(PulsarWorkerService.java:208) at org.apache.pulsar.functions.worker.Worker.start(Worker.java:54) at org.apache.pulsar.functions.worker.FunctionWorkerStarter.main(FunctionWorkerStarter.java:76) ``` Additionaly there's the same issue if we upgrade the broker before the fn worker: 1. the broker gets the upgrade. won't serve `zookeeperServers` field 2. fn worker restarts for some reasons. 3. fn worker gets the internal config and look for `zookeeperServers` field which is empty in the json 4. NPE * Restore old fields in `InternalConfigurationData` and add fallback the old values in the new fields getters * Added unit test - [x] `doc-not-needed` (cherry picked from commit 1a34b87) (cherry picked from commit 0993a34)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
zookeeperServersalready deprecated, we recommendmetadataStoreUrl.In
BookKeeperPackagesStorage, it still usezookeeperServers, didn't compatible withmetadataStoreUrl, so it will throw NPE.I notice that there are two PRs already fix it, but it only in Branch-2.11, so move it to Branch-2.10.
#14384, #14595
Documentation
Check the box below or label this PR directly.
Need to update docs?
doc-required(Your PR needs to update docs and you will update later)
doc-not-needed(Please explain why)
doc(Your PR contains doc changes)
doc-complete(Docs have been already added)