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

Make standalone use setMetadataStoreUrl #14384

Merged

Conversation

gaozhangmin
Copy link
Contributor

@gaozhangmin gaozhangmin commented Feb 19, 2022

Motivation

Remove deprecated zookeeperServers in standalone.
Fix incorrect ZK connection String after metadataStoreUrl added ZK prefix.

Modifications

setZookeeperServers to setMetadataStoreUrl

Documentation

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

@github-actions
Copy link

@gaozhangmin: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

@gaozhangmin:Thanks for providing doc info!

@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Feb 19, 2022
@gaozhangmin gaozhangmin force-pushed the standalone-setMetadataStoreUrl branch from b467605 to 3c4dcc2 Compare February 21, 2022 06:55
@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@codelipenghui codelipenghui added this to the 2.11.0 milestone Feb 23, 2022
@gaozhangmin gaozhangmin force-pushed the standalone-setMetadataStoreUrl branch from 7617a36 to ae4c657 Compare February 25, 2022 06:11
@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaozhangmin gaozhangmin force-pushed the standalone-setMetadataStoreUrl branch from ae4c657 to 628515b Compare February 25, 2022 07:25
@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaozhangmin gaozhangmin force-pushed the standalone-setMetadataStoreUrl branch from 1d1b3d5 to 098be8c Compare February 25, 2022 11:01
@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaozhangmin gaozhangmin force-pushed the standalone-setMetadataStoreUrl branch 2 times, most recently from f51129b to 67a5f6f Compare February 28, 2022 06:29
@gaozhangmin gaozhangmin force-pushed the standalone-setMetadataStoreUrl branch from f18c847 to a26f647 Compare February 28, 2022 08:16
@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaozhangmin
Copy link
Contributor Author

gaozhangmin commented Feb 28, 2022

@nicoloboschi there were backward compatibility problem before, I fixed it. PTAL again. Thx

if (StringUtils.isNotBlank(configuration.getMetadataStoreUrl())) {
                ledgersStoreServers = configuration.getMetadataStoreUrl();
                if (ledgersStoreServers.startsWith(ZK_SCHEME_IDENTIFIER)) {
                    ledgersStoreServers = ledgersStoreServers.substring(ZK_SCHEME_IDENTIFIER.length());
                }
            } else {
                ledgersStoreServers = configuration.getZookeeperServers();
            }

@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit e20f34b into apache:master Mar 1, 2022
@gaoran10 gaoran10 changed the title Make stadnalone use setMetadataStoreUrl Make standalone use setMetadataStoreUrl Mar 7, 2022
gaozhangmin pushed a commit to gaozhangmin/pulsar that referenced this pull request Mar 8, 2022
codelipenghui pushed a commit that referenced this pull request Mar 9, 2022
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
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
Labels
doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants