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.

Sign up for GitHub

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

Jump to bottom

[Broker] Fix read schema compatibility strategy priority #13938

Merged
merged 1 commit into from Jan 26, 2022
Merged

[Broker] Fix read schema compatibility strategy priority #13938

merged 1 commit into from Jan 26, 2022

Conversation

nodece
Copy link
Member

@nodece nodece commented Jan 25, 2022 •
edited

Signed-off-by: Zixuan Liu nodeces@gmail.com

Motivation

When we defined the schemaCompatibilityStrategy=ALWAYS_INCOMPATIBLE in broker.conf, and a namespace policies so like:

schema_auto_update_compatibility_strategy = SchemaAutoUpdateCompatibilityStrategy.Full
schema_compatibility_strategy = null

We should get SchemaCompatibilityStrategy.FULL by pulsar-admin namespaces get-schema-compatibility-strategy <ns>, but got SchemaCompatibilityStrategy.ALWAYS_INCOMPATIBLE, this is incorrect response.

Modifications

  • Use null as Policies#schema_auto_update_compatibility_strategy value
  • Use FULL as schemaCompatibilityStrategy value in broker.conf and update configuration code
  • Only return Policies#schema_compatibility_strategy when get schema compatibility strategy of namespace
  • Change the read schema compatibility strategy priority

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

@codelipenghui codelipenghui added this to the 2.10.0 milestone Jan 25, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 25, 2022
@nodece
Copy link
Member Author

nodece commented Jan 25, 2022

Hi @Jason918, could you take a look this?

@nodece nodece force-pushed the fix_schema_compatibility_strategy branch 2 times, most recently from 599c6f6 to 39e7749 Compare January 25, 2022 10:56
@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Jan 25, 2022
@nodece nodece force-pushed the fix_schema_compatibility_strategy branch from 39e7749 to 3e76c3e Compare January 25, 2022 17:03
@nodece nodece force-pushed the fix_schema_compatibility_strategy branch 3 times, most recently from 74c508f to 67719fe Compare January 26, 2022 02:49
@nodece
Copy link
Member Author

nodece commented Jan 26, 2022

/pulsarbot rerun-failure-checks

@nodece
[Broker] Fix read schema compatibility strategy priority
9282042 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece nodece force-pushed the fix_schema_compatibility_strategy branch from 67719fe to 9282042 Compare January 26, 2022 04:21
congbobo184
congbobo184 approved these changes Jan 26, 2022
View reviewed changes
Copy link
Contributor

@congbobo184 congbobo184 left a comment

Choose a reason for hiding this comment

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

LGTM

@codelipenghui codelipenghui merged commit c18d645 into apache:master Jan 26, 2022
codelipenghui pushed a commit that referenced this pull request Jan 27, 2022
@nodece @codelipenghui
[Broker] Fix read schema compatibility strategy priority (#13938)
9601de1 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

When we defined the `schemaCompatibilityStrategy=ALWAYS_INCOMPATIBLE` in `broker.conf`, and a namespace policies so like:
```
schema_auto_update_compatibility_strategy = SchemaAutoUpdateCompatibilityStrategy.Full
schema_compatibility_strategy = null
```
We should get `SchemaCompatibilityStrategy.FULL` by `pulsar-admin namespaces get-schema-compatibility-strategy <ns>`, but got `SchemaCompatibilityStrategy.ALWAYS_INCOMPATIBLE`, this is incorrect response.

- Use `null` as `Policies#schema_auto_update_compatibility_strategy` value
- Use `FULL` as `schemaCompatibilityStrategy` value in `broker.conf` and update configuration code
- Only return `Policies#schema_compatibility_strategy` when get schema compatibility strategy of namespace
- Change the read schema compatibility strategy priority

(cherry picked from commit c18d645)
@codelipenghui codelipenghui added cherry-picked/branch-2.8 Archived: 2.8 is end of life and removed cherry-picked/branch-2.8 Archived: 2.8 is end of life labels Jan 27, 2022
codelipenghui pushed a commit that referenced this pull request Jan 29, 2022
@nodece @codelipenghui
[Broker] Fix read schema compatibility strategy priority (#13938)
44d3c59 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

### Motivation

When we defined the `schemaCompatibilityStrategy=ALWAYS_INCOMPATIBLE` in `broker.conf`, and a namespace policies so like:
```
schema_auto_update_compatibility_strategy = SchemaAutoUpdateCompatibilityStrategy.Full
schema_compatibility_strategy = null
```
We should get `SchemaCompatibilityStrategy.FULL` by `pulsar-admin namespaces get-schema-compatibility-strategy <ns>`, but got `SchemaCompatibilityStrategy.ALWAYS_INCOMPATIBLE`, this is incorrect response.

### Modifications

- Use `null` as `Policies#schema_auto_update_compatibility_strategy` value
- Use `FULL` as `schemaCompatibilityStrategy` value in `broker.conf` and update configuration code
- Only return `Policies#schema_compatibility_strategy` when get schema compatibility strategy of namespace
- Change the read schema compatibility strategy priority

(cherry picked from commit c18d645)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Jan 29, 2022
zymap pushed a commit that referenced this pull request Jan 30, 2022
@nodece @zymap
[Broker] Fix read schema compatibility strategy priority (#13938)
dab6014 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

When we defined the `schemaCompatibilityStrategy=ALWAYS_INCOMPATIBLE` in `broker.conf`, and a namespace policies so like:
```
schema_auto_update_compatibility_strategy = SchemaAutoUpdateCompatibilityStrategy.Full
schema_compatibility_strategy = null
```
We should get `SchemaCompatibilityStrategy.FULL` by `pulsar-admin namespaces get-schema-compatibility-strategy <ns>`, but got `SchemaCompatibilityStrategy.ALWAYS_INCOMPATIBLE`, this is incorrect response.

- Use `null` as `Policies#schema_auto_update_compatibility_strategy` value
- Use `FULL` as `schemaCompatibilityStrategy` value in `broker.conf` and update configuration code
- Only return `Policies#schema_compatibility_strategy` when get schema compatibility strategy of namespace
- Change the read schema compatibility strategy priority

(cherry picked from commit c18d645)
@zymap zymap added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Jan 30, 2022
@Anonymitaet
Copy link
Member

Anonymitaet commented Feb 7, 2022 •
edited

@nodece are the docs added in #13983? If yes, can you link the code and the corresponding doc PR in the future? (so that others can see the changes clearly, thanks).

@nodece
Copy link
Member Author

nodece commented Feb 7, 2022

@Anonymitaet Done, thanks.

@Anonymitaet Anonymitaet added doc-complete Your PR changes impact docs and the related docs have been already added. and removed doc-required Your PR changes impact docs and you will update later. labels Feb 7, 2022
@nodece nodece mentioned this pull request Feb 7, 2022
Merged
1 task
@rdhabalia rdhabalia mentioned this pull request Nov 30, 2022
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codelipenghui codelipenghui codelipenghui approved these changes

@congbobo184 congbobo184 congbobo184 approved these changes

Assignees

@nodece nodece

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-complete Your PR changes impact docs and the related docs have been already added. doc-not-needed Your PR changes do not impact docs release/2.8.3 release/2.9.2
Projects
None yet
Milestone
2.10.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@nodece @Anonymitaet @codelipenghui @congbobo184 @gaoran10 @zymap