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

[Issue 5562][docs]Modify schema compatibility check doc #5757

Merged

Conversation

@congbobo184
Copy link
Contributor

congbobo184 commented Nov 27, 2019

Fixes #5562
In order to add the doc for #5227 , to solve the issue #5562

Motivation

modify the doc.

@congbobo184

This comment has been minimized.

Copy link
Contributor Author

congbobo184 commented Nov 27, 2019

@Anonymitaet

This comment has been minimized.

Copy link
Member

Anonymitaet commented Nov 27, 2019

@congbobo184 I'll review later.

congbobo added 5 commits Nov 28, 2019
@congbobo184

This comment has been minimized.

Copy link
Contributor Author

congbobo184 commented Nov 28, 2019

run cpp tests
run java8 tests


<td>

The schema compatibility check strategy in order to Upgrade to 2.5.0 or above for compatibility with pulsar version below 2.5.0.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
The schema compatibility check strategy in order to Upgrade to 2.5.0 or above for compatibility with pulsar version below 2.5.0.
* If you upgrade a lower version to Pulsar 2.5.0, then it uses the `UNDEFINED` strategy, at this time, it actually uses the compatibility strategy of the topic.
* If you use Pulsar 2.5.0 and a higher version, then it uses the `UNDEFINED` strategy, at this time, it actually uses the `FULL` compatibility strategy.

<td>

NONE

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
NONE
It depends

NONE

</td>

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
</td>
It depends

<td>

NONE

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
NONE
It depends

<th>

Note

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
Note
Description

<td>

The schema compatibility check strategy in order to Upgrade to 2.5.0 or above for compatibility with pulsar version below 2.5.0.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
The schema compatibility check strategy in order to Upgrade to 2.5.0 or above for compatibility with pulsar version below 2.5.0.
* If you upgrade a lower version to Pulsar 2.5.0, then it uses the `UNDEFINED` strategy, at this time, it actually uses the compatibility strategy of the topic.
* If you use Pulsar 2.5.0 and a higher version, then it uses the `UNDEFINED` strategy, at this time, it actually uses the `FULL` compatibility strategy.
@@ -781,3 +853,168 @@ Consequently, you can upgrade the producers and consumers in **any order**.
</tr>

</table>

## Schema verification for producers

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member

I've moved this part before order of upgrading clients, please delete lines 857-1017.

</tr>

</table>

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member

Add schema verification here. I've re-written some sentences as well.

Suggested change
## Schema verification
When a producer or a consumer tries to connect to a topic, a broker performs some checks to verify a schema.
### Producer
When a producer tries to connect to a topic (suppose ignore the schema auto creation), a broker does the following checks:
* Check if the schema carried by the producer exists in the schema registry or not.
* If the schema is already registered, then the producer is connected to a broker and produce messages with that schema.
* If the schema is not registered, then Pulsar verifies if the schema is allowed to be registered based on the configured compatibility check strategy.
### Consumer
When a consumer tries to connect to a topic, a broker checks if a carried schema is compatible with a registered schema based on the configured schema compatibility check strategy.
<table style="table">
<tr>
<th>
Compatibility check strategy
</th>
<th>
Check logic
</th>
</tr>
<tr>
<td>
`ALWAYS_COMPATIBLE`
</td>
<td>
All pass
</td>
</tr>
<tr>
<td>
`ALWAYS_INCOMPATIBLE`
</td>
<td>
No pass
</td>
</tr>
<tr>
<td>
`BACKWARD`
</td>
<td>
Can read the last schema
</td>
</tr>
<tr>
<td>
`BACKWARD_TRANSITIVE`
</td>
<td>
Can read all schemas
</td>
</tr>
<tr>
<td>
`FORWARD`
</td>
<td>
Can read the last schema
</td>
</tr>
<tr>
<td>
`FORWARD_TRANSITIVE`
</td>
<td>
Can read the last schema
</td>
</tr>
<tr>
<td>
`FULL`
</td>
<td>
Can read the last schema
</td>
</tr>
<tr>
<td>
`FULL_TRANSITIVE`
</td>
<td>
Can read all schemas
</td>
</tr>
</table>
@congbobo184

This comment has been minimized.

Copy link
Contributor Author

congbobo184 commented Nov 29, 2019

@sijie

bin/pulsar-admin namespaces set-schema-autoupdate-strategy --compatibility <compatibility-level> tenant/namespace
bin/pulsar-admin namespaces get-schema-autoupdate-strategy --compatibility <compatibility-level> tenant/namespace

I think this two command should not be provided to users.
It's easy to confuse users following commands :

bin/pulsar-admin namespaces set-schema-compatibility-strategy --compatibility <compatibility-level> tenant/namespace
bin/pulsar-admin namespaces get-schema-compatibility-strategy --compatibility <compatibility-level> tenant/namespace
bin/pulsar-admin namespaces get-is-allow-auto-update-schema --disable tenant/namespace
@sijie

This comment has been minimized.

Copy link
Member

sijie commented Nov 29, 2019

@congbobo184 if those two commands are deprecated, you can add hidden flag to hide them.

@congbobo184

This comment has been minimized.

Copy link
Contributor Author

congbobo184 commented Nov 29, 2019

This seems like a good way.

* If it is a new schema and it passes the compatibility check, the broker registers a new schema automatically for the topic.

* If the schema does not pass the compatibility check, the broker does not register a schema.
* If the schema is registered, producer is connected.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
* If the schema is registered, producer is connected.
* If the schema is registered, a producer is connected to a broker.
* If the schema does not pass the compatibility check, the broker does not register a schema.
* If the schema is registered, producer is connected.

* If the schema isn't registered

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
* If the schema isn't registered
* If the schema is not registered:

* If the schema isn't registered

* If`isAllowAutoUpdateSchema` = false, producer is rejected.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
* If`isAllowAutoUpdateSchema` = false, producer is rejected.
* If `isAllowAutoUpdateSchema` sets to **false**, the producer is rejected to connect to a broker.

* If`isAllowAutoUpdateSchema` = false, producer is rejected.

* If `isAllowAutoUpdateSchema` = true

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
* If `isAllowAutoUpdateSchema` = true
* If `isAllowAutoUpdateSchema` sets to **true**:

* If `isAllowAutoUpdateSchema` = true

* If schema pass the compatibility check then the broker registers a new schema automatically for the topic and producer is connected.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
* If schema pass the compatibility check then the broker registers a new schema automatically for the topic and producer is connected.
* If schema pass the compatibility check then the broker registers a new schema automatically for the topic and producer is connected.
Suggested change
* If schema pass the compatibility check then the broker registers a new schema automatically for the topic and producer is connected.
* If the schema passes the compatibility check, then the broker registers a new schema automatically for the topic and the producer is connected.
* If the **schema does not pass the compatibility check**, the consumer is rejected and disconnected.

![AutoUpdate Producer](assets/schema-autoupdate-consumer.png)
* If schema pass the compatibility check, the consumer is connected.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
* If schema pass the compatibility check, the consumer is connected.
* If the schema passes the compatibility check, then the consumer is connected to the broker.
![AutoUpdate Producer](assets/schema-autoupdate-consumer.png)
* If schema pass the compatibility check, the consumer is connected.

* If schema does not pass the compatibility, the consumer is rejected.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
* If schema does not pass the compatibility, the consumer is rejected.
* If the schema does not pass the compatibility check, then the consumer is rejected to connect to the broker.
@@ -75,7 +88,7 @@ You can use the `pulsar-admin` command to manage the `AutoUpdate` strategy as be
To disable `AutoUpdate` on a namespace, you can use the `pulsar-admin` command.

```bash
bin/pulsar-admin namespaces set-schema-autoupdate-strategy --disabled tenant/namespace
bin/pulsar-admin namespaces set-is-allow-auto-update-schema --disabled tenant/namespace

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member

Do we consider adding enable AutoUpdate before this part?

From line 82:

Suggested change
bin/pulsar-admin namespaces set-is-allow-auto-update-schema --disabled tenant/namespace
* [Enable AutoUpdate](#enable-autoupdate)
* [Disable AutoUpdate](#disable-autoupdate)
* [Adjust compatibility](#adjust-compatibility)
#### Enable AutoUpdate
To enable `AutoUpdate` on a namespace, you can use the `pulsar-admin` command.
```bash
bin/pulsar-admin namespaces set-is-allow-auto-update-schema --enable tenant/namespace
5. If no, the broker judge whether this namespace can automatically create schema.
isAllowAutoUpdateSchema can be modified by Admin-Api and REST-API
Comment on lines 543 to 544

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
5. If no, the broker judge whether this namespace can automatically create schema.
isAllowAutoUpdateSchema can be modified by Admin-Api and REST-API
5. If no, the broker verifies whether a schema can be automatically created in this namespace:
* If `isAllowAutoUpdateSchema` sets to **true**, then a schema can be created, and the broker validates the schema based on the schema compatibility check strategy defined for the topic.
* If `isAllowAutoUpdateSchema` sets to **false**, then a schema can not be created, and the consumer is rejected to connect to the broker.
**Tip**:
`isAllowAutoUpdateSchema` can be set via **Pulsar admin API** or **REST API.**
For how to set `isAllowAutoUpdateSchema` via Pulsar admin API, see [Manage AutoUpdate Strategy](schema-manage.md/#manage-autoupdate-strategy).

5. If no, the broker validates the schema based on the schema compatibility check strategy defined for the topic.
6. If yes, the broker validates the schema based on the schema compatibility check strategy defined for the topic.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member

I've moved the line 546 to the step 5.
If it is correct, can we delete the line 546?

image

This comment has been minimized.

Copy link
@congbobo184

congbobo184 Nov 29, 2019

Author Contributor

yes, we can delete the line 546 👍 :)

5. If no, the broker validates the schema based on the schema compatibility check strategy defined for the topic.

6. If the schema is compatible, the broker stores it and returns the schema version to the consumer.
3. If the schema is compatible, the consumer will be connected.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
3. If the schema is compatible, the consumer will be connected.
3. If the schema is compatible, the consumer is connected to the broker.

7. If the schema is incompatible, the consumer will be disconnected.
4. If the schema is incompatible, the consumer will be disconnected.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
4. If the schema is incompatible, the consumer will be disconnected.
4. If the schema is incompatible, the consumer is disconnected to the broker.
@Anonymitaet

This comment has been minimized.

Copy link
Member

Anonymitaet commented Nov 29, 2019

@congbobo184 as we discussed before, we finalize contents first and then re-draw the workflow, so pls do not add images now.

@congbobo184

This comment has been minimized.

Copy link
Contributor Author

congbobo184 commented Nov 29, 2019

@sijie where does the flag add? pulsar-client-tools or broker reject this request? if it is in pular-client-tools, how to add it?

@congbobo184

This comment has been minimized.

Copy link
Contributor Author

congbobo184 commented Nov 29, 2019

Thanks, i will look carefully and add hidden flag to hide them. :)

congbobo added 3 commits Nov 29, 2019
@@ -46,7 +46,7 @@ For more information, see [Schema compatibility check strategy](#schema-compatib

## Schema compatibility check strategy

Pulsar has 8 schema compatibility check strategies, which are summarized in the following table.
Pulsar has 9 schema compatibility check strategies, which are summarized in the following table.

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member
Suggested change
Pulsar has 9 schema compatibility check strategies, which are summarized in the following table.
Pulsar has 8 schema compatibility check strategies, which are summarized in the following table.
* [Enable AutoUpdate](#enable-autoupdate)

* [Disable AutoUpdate](#disable-autoupdate)
Comment on lines +82 to 84

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member

As we discussed just now, please correct the commands for enable and disable


`isAllowAutoUpdateSchema` can be set via **Pulsar admin API** or **REST API.**

For how to set `isAllowAutoUpdateSchema` via Pulsar admin API, see [Manage AutoUpdate Strategy](schema-manage.md/#manage-autoupdate-strategy).

This comment has been minimized.

Copy link
@Anonymitaet

Anonymitaet Nov 29, 2019

Member

Seems a little weird between #5 and #6? Do we need to add some contents like "If a schema is updated automatically:” after #5?

Copy link
Member

Anonymitaet left a comment

PTAL

congbobo added 2 commits Dec 2, 2019
@congbobo184

This comment has been minimized.

Copy link
Contributor Author

congbobo184 commented Dec 2, 2019

run cpp tests

@congbobo184

This comment has been minimized.

Copy link
Contributor Author

congbobo184 commented Dec 2, 2019

run Integration Tests

@Jennifer88huang Jennifer88huang changed the title [doc]Modify schema compatibility check doc [Issue 5562][docs]Modify schema compatibility check doc Dec 3, 2019
@Jennifer88huang

This comment has been minimized.

Copy link
Member

Jennifer88huang commented Dec 3, 2019

@congbobo184 @Anonymitaet When you fix an issue, please add the issue number and component in the title, for example, "[Issue 5562][docs] xxx", and add "Fixes #xxxx" in the description.
So when the PR is merged, the corresponding issue will be closed automatically.

@sijie
sijie approved these changes Dec 3, 2019
@sijie

This comment has been minimized.

Copy link
Member

sijie commented Dec 3, 2019

@congbobo184 @Anonymitaet When you fix an issue, please add the issue number and component in the title, for example, "[Issue 5562][docs] xxx", and add "Fixes #xxxx" in the description.
So when the PR is merged, the corresponding issue will be closed automatically.

maybe we can add this to the contributor guide and encourage committers and contributors following the same practice.

@sijie
sijie approved these changes Dec 3, 2019
@sijie sijie merged commit 180e28a into apache:master Dec 3, 2019
3 of 4 checks passed
3 of 4 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
Jenkins: C++ / Python Tests SUCCESS
Details
Jenkins: Integration Tests SUCCESS
Details
Jenkins: Java 8 - Unit Tests SUCCESS
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.