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

[PIP-44] Separate schema compatibility checker for producer and consumer #5227

Merged
merged 38 commits into from Nov 4, 2019
Merged

[PIP-44] Separate schema compatibility checker for producer and consumer #5227

merged 38 commits into from Nov 4, 2019

Conversation

congbobo184
Copy link
Contributor

@congbobo184 congbobo184 commented Sep 19, 2019

Master Issue: #4737

Motivation

Implement of PIP-44.

Modifications

  • Add two flag :
  1. schema_auto_update_compatibility_strategy for whether producer automatically creates schema under compatibility checking conditions
  2. schema_compatibility_strategy for pulsar schema compatibility check
  • Consumer and producer has their own schema compatibility check, specific in PIP-44

Verifying this change

Add the tests for it

Does this pull request potentially affect one of the following parts:
If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): (no)
The public API: (no)
The schema: (yes)
The default values of configurations: (no)
The wire protocol: (no)
The rest endpoints: (no)
The admin cli options: (yes)
Anything that affects deployment: (no)

Documentation

Does this pull request introduce a new feature? (yes)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
If a feature is not applicable for documentation, explain why?
If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@sijie
Copy link
Member

sijie commented Sep 19, 2019

@congbobo184 isn't #5165 already doing the same thing?

@congbobo184 congbobo184 changed the title [PIP-43] Support producer to send msg with different schema [PIP-44] Separate schema compatibility checker for producer and consumer Sep 20, 2019
@congbobo184
Copy link
Contributor Author

run cpp tests

@sijie
Copy link
Member

sijie commented Oct 14, 2019

@congbobo184 @codelipenghui

I see you introduced two settings, one is the flag to control auto update and the other flag is to set the compatibility check strategy. These two settings are fine. I am just wondering how are we going to do with the existing auto update schema strategy. Are we going to deprecated the existing one?

@congbobo184 congbobo184 reopened this Oct 15, 2019
@codelipenghui
Copy link
Contributor

@sijie if user set the auto update strategy by new way, we will use the new setting, otherwise will use old setting. So this change can read the old setting, if user update the auto update strategy by new way and then update it by the old way(May be a different version of the broker), new change will be ignored.

@codelipenghui
Copy link
Contributor

run Integration Tests
run java8 tests

# Conflicts:
#	pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
#	pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
@wolfstudy
Copy link
Member

ping @sijie PTAL

@sijie
Copy link
Member

sijie commented Oct 28, 2019

@codelipenghui @congbobo184 can you please update the schema documentation to reflect to this change? @Anonymitaet can help review the content, since she has written all the schema documentation for Pulsar.

# Conflicts:
#	pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/SchemaRegistryServiceImpl.java
@congbobo184
Copy link
Contributor Author

run java8 tests

@sijie sijie merged commit f6701f1 into apache:master Nov 4, 2019
@sijie
Copy link
Member

sijie commented Nov 4, 2019

@congbobo184 @codelipenghui can you guys add documentation about this? /cc @jennifer88huang @Anonymitaet for this. Make sure the documentation for this feature goes in before we release 2.5.0.

@congbobo184
Copy link
Contributor Author

OK, I will add the documentation about this. :)

@sijie
Copy link
Member

sijie commented Nov 5, 2019

@congbobo184 can you create a github issue for that?

@Jennifer88huang-zz
Copy link
Contributor

Got it.

sijie pushed a commit that referenced this pull request Dec 3, 2019
Fixes #5562 

Update the schema compatibility check documentation to follow the latest changes from #5227
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants