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

NPE when get isAllowAutoUploadSchema #13831

Merged

Conversation

gaozhangmin
Copy link
Contributor

@gaozhangmin gaozhangmin commented Jan 19, 2022

Motivation

there were some potential NPE bug when get null value by pulsar-admin

Caused by: java.lang.NullPointerException
        at org.apache.pulsar.broker.admin.impl.NamespacesBase.internalGetIsAllowAutoUpdateSchema(NamespacesBase.java:2423) ~[org.apache.pulsar-pulsar-broker-2.9.1.jar:2.9.1]
        at org.apache.pulsar.broker.admin.v2.Namespaces.getIsAllowAutoUpdateSchema(Namespaces.java:1631) ~[org.apache.pulsar-pulsar-broker-2.9.1.jar:2.9.1]
        at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]

Modifications

1、set default value true to is_allow_auto_update_schema in Policies as same as ServiceConfiguration
2、changes from boolean to Boolean, long to Long, prevent potential NPE.

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 the doc-not-needed Your PR changes do not impact docs label Jan 19, 2022
@shoothzj
Copy link
Member

I think that we may make a break change of client API.
before we change: client throws an exception
after we change: client raised an NPE

Am I correct?
cc @codelipenghui @mattisonchao

@gaozhangmin
Copy link
Contributor Author

I think that we may make a break change of client API. before we change: client throws an exception after we change: client raised an NPE

Am I correct? cc @codelipenghui @mattisonchao

I think print null value by pulsar-admin client has no problem We don't need to make this change.

@gaozhangmin gaozhangmin force-pushed the npe-get-is-allow-auto-update-schema branch from 4eb3557 to 936c21a Compare January 19, 2022 11:53
@shoothzj
Copy link
Member

I think that we may make a break change of client API. before we change: client throws an exception after we change: client raised an NPE
Am I correct? cc @codelipenghui @mattisonchao

I think print null value by pulsar-admin client has no problem We don't need to make this change.

People can use pulsar admin not only in terminal, can use it in code.

@gaozhangmin gaozhangmin force-pushed the npe-get-is-allow-auto-update-schema branch from 936c21a to 54c8801 Compare January 20, 2022 03:14
@gaozhangmin
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaozhangmin
Copy link
Contributor Author

@codelipenghui PTAL

@codelipenghui
Copy link
Contributor

@gaozhangmin Looks like #12786 not released yet) introduced the problem, but from the stack you have provided, looks like it happens in 2.9.1.

@codelipenghui codelipenghui merged commit 0a8794b into apache:master Jan 21, 2022
codelipenghui pushed a commit that referenced this pull request Jan 21, 2022
### Motivation

there were some potential NPE bug when get null value by pulsar-admin
```
Caused by: java.lang.NullPointerException
        at org.apache.pulsar.broker.admin.impl.NamespacesBase.internalGetIsAllowAutoUpdateSchema(NamespacesBase.java:2423) ~[org.apache.pulsar-pulsar-broker-2.9.1.jar:2.9.1]
        at org.apache.pulsar.broker.admin.v2.Namespaces.getIsAllowAutoUpdateSchema(Namespaces.java:1631) ~[org.apache.pulsar-pulsar-broker-2.9.1.jar:2.9.1]
        at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
```

### Modifications
1、set default value true to is_allow_auto_update_schema in Policies as same as ServiceConfiguration
2、changes from boolean to Boolean, long  to Long, prevent potential NPE.

(cherry picked from commit 0a8794b)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Jan 21, 2022
@gaoran10 gaoran10 added area/broker area/admin type/bug The PR fixed a bug or issue reported a bug and removed area/broker area/admin labels Jan 25, 2022
codelipenghui pushed a commit that referenced this pull request Jan 27, 2022
### Motivation

there were some potential NPE bug when get null value by pulsar-admin
```
Caused by: java.lang.NullPointerException
        at org.apache.pulsar.broker.admin.impl.NamespacesBase.internalGetIsAllowAutoUpdateSchema(NamespacesBase.java:2423) ~[org.apache.pulsar-pulsar-broker-2.9.1.jar:2.9.1]
        at org.apache.pulsar.broker.admin.v2.Namespaces.getIsAllowAutoUpdateSchema(Namespaces.java:1631) ~[org.apache.pulsar-pulsar-broker-2.9.1.jar:2.9.1]
        at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
```

### Modifications
1、set default value true to is_allow_auto_update_schema in Policies as same as ServiceConfiguration
2、changes from boolean to Boolean, long  to Long, prevent potential NPE.

(cherry picked from commit 0a8794b)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.3 release/2.9.2 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants