Skip to content

[schema] KeyValue schema support using AUTO_CONSUME as key/value schema #4839

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

Merged
merged 4 commits into from
Jul 30, 2019

Conversation

sijie
Copy link
Member

@sijie sijie commented Jul 28, 2019

Motivation

Currently KeyValue schema doesn't support using AUTO_CONSUME.

This PR is to add this support.

This PR is based on #4836

Changes

  • refactor a bit on Schema interface to support fetching schema info for both AutoConsumeSchema and KeyValueSchema before subscribing
  • add AUTO_CONSUME support to KeyValueSchema
  • add tests

sijie added 2 commits July 28, 2019 12:24

Unverified

This user has not yet uploaded their public signing key.
*Motivation*

Currently fetching schema information is done synchronously.
It is called in netty callback threads and will potentially block
async operations.

*Modifications*

Make most of the operations asynchronously in SchemaInfoProvider.

Unverified

This user has not yet uploaded their public signing key.
*Motivation*

Currently KeyValue schema doesn't support using AUTO_CONSUME.

This PR is to add this support.

This PR is based on apache#4836

*Changes*

- refactor a bit on Schema interface to support fetching schema info for both AutoConsumeSchema and KeyValueSchema before subscribing
- add AUTO_CONSUME support to KeyValueSchema
- add tests
@sijie sijie added type/feature The PR added a new feature or issue requested a new feature component/schemaregistry labels Jul 28, 2019
@sijie sijie added this to the 2.5.0 milestone Jul 28, 2019
@sijie sijie self-assigned this Jul 28, 2019

Unverified

This user has not yet uploaded their public signing key.
Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, just left a comment relate to unit test.

.topic(topic)
.subscriptionName("sub2")
.subscriptionInitialPosition(SubscriptionInitialPosition.Earliest)
.subscribe();
Copy link
Contributor

Choose a reason for hiding this comment

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

Will c2 be reject by broker? default is use exclusive subscription mode.

Copy link
Member Author

Choose a reason for hiding this comment

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

No because they are using different subscription names

Unverified

This user has not yet uploaded their public signing key.
…port_autoschema
@codelipenghui
Copy link
Contributor

run Integration Tests

@sijie sijie merged commit dd7cc89 into apache:master Jul 30, 2019
@sijie sijie deleted the keyvalue_schema_support_autoschema branch July 30, 2019 14:41
@jiazhai jiazhai modified the milestones: 2.5.0, 2.4.1 Aug 28, 2019
@jiazhai
Copy link
Member

jiazhai commented Aug 28, 2019

mark 2.4.1 for conflict #4962

jiazhai pushed a commit that referenced this pull request Aug 28, 2019
…ema (#4839)

*Motivation*

Currently KeyValue schema doesn't support using AUTO_CONSUME.

This PR is to add this support.

This PR is based on #4836

*Changes*

- refactor a bit on Schema interface to support fetching schema info for both AutoConsumeSchema and KeyValueSchema before subscribing
- add AUTO_CONSUME support to KeyValueSchema
- add tests
(cherry picked from commit dd7cc89)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants