Skip to content

add async commit downgrading design doc - #93

Open
sticnarf wants to merge 1 commit into
tikv:masterfrom
sticnarf:async-commit-downgrade
Open

add async commit downgrading design doc#93
sticnarf wants to merge 1 commit into
tikv:masterfrom
sticnarf:async-commit-downgrade

Conversation

@sticnarf

@sticnarf sticnarf commented Feb 2, 2021

Copy link
Copy Markdown
Contributor

It is likely we need to provide a solution for downgrading from TiKV 5.0 to 4.0. Here is the doc about handling async commit.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

After resolving locks, there should be no locks using the async commit protocol. And because async commit has already been disabled, no more async commit locks will appear.

3. Wait until all peers apply to the current index to guarantee that no async commit locks or no write records with `FLAG_OVERLAPPED_ROLLBACK` or `gc_fence` will be written to any of the TiKV stores.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How to do it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Iterate all peers and do a follower read (execute a read index) for each, then I think we can make sure it's up to date.

@youjiali1995

Copy link
Copy Markdown
Contributor

The solution sounds fine to me. We don't need to do it now, because we haven't figured out the requirements of downgrading. For example, even if this feature can downgrade without stop, other features like clustered index can't downgrade in place. If we have to downgrade by data migration, we don't need to do it.

@nrc

nrc commented Feb 4, 2021

Copy link
Copy Markdown
Collaborator

What would happen if we just leave the extra fields? I assumed older TiKV would just ignore them? If not can we write a small patch to do that and backport it?

@nrc

nrc commented Feb 4, 2021

Copy link
Copy Markdown
Collaborator

Otherwise, lgtm

@sticnarf

sticnarf commented Feb 4, 2021

Copy link
Copy Markdown
Contributor Author

What would happen if we just leave the extra fields? I assumed older TiKV would just ignore them? If not can we write a small patch to do that and backport it?

All 4.0 versions now panic when seeing these fields. I worry even if we patch the latest 4.0 version, the users may still meet problems when they do a second downgrade to an even older 4.0 version... (We promise patch versions are compatible with each other)

@nrc

nrc commented Feb 4, 2021

Copy link
Copy Markdown
Collaborator

What would happen if we just leave the extra fields? I assumed older TiKV would just ignore them? If not can we write a small patch to do that and backport it?

All 4.0 versions now panic when seeing these fields. I worry even if we patch the latest 4.0 version, the users may still meet problems when they do a second downgrade to an even older 4.0 version... (We promise patch versions are compatible with each other)

Hmm, this seems like a problem, we should discuss with product, but it seems to me that we need to make TiKV forwards compatible in this kind of way and only offer back compat guarantees up to versions which have forwards compat built-in.

@youjiali1995

Copy link
Copy Markdown
Contributor

What would happen if we just leave the extra fields? I assumed older TiKV would just ignore them? If not can we write a small patch to do that and backport it?

All 4.0 versions now panic when seeing these fields. I worry even if we patch the latest 4.0 version, the users may still meet problems when they do a second downgrade to an even older 4.0 version... (We promise patch versions are compatible with each other)

We can ignore extra fields since v5.0. :)

ti-chi-bot pushed a commit to tikv/tikv that referenced this pull request Mar 4, 2021
Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

<!--
Thank you for contributing to TiKV!

If you haven't already, please read TiKV's [CONTRIBUTING](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md) document.

If you're unsure about anything, just ask; somebody should be along to answer within a day or two.

PR Title Format:
1. module [, module2, module3]: what's changed
2. *: what's changed

If you want to open the **Challenge Program** pull request, please use the following template:
https://raw.githubusercontent.com/tikv/.github/master/.github/PULL_REQUEST_TEMPLATE/challenge-program.md
You can use it with query parameters: https://github.com/tikv/tikv/compare/master...${you branch}?template=challenge-program.md
-->

### What problem does this PR solve?

Problem Summary:

To make downgrading easier, Ignore unknown bytes when parsing Write/Lock. tikv/sig-transaction#93

### What is changed and how it works?

What's Changed:

Ignore unknown bytes when parsing Write/Lock.

### Related changes


### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

- Unit test

### Release note <!-- bugfixes or new feature need a release note -->
- No release note.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants