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

*: add a procedure that upgrades data from v2 to v3 #4381

Merged
merged 9 commits into from Mar 22, 2019

Conversation

overvenus
Copy link
Member

What have you changed? (mandatory)

In v3.x, we move all meta data to raft engine and remove RAFT CF of kv engine. This is a precede change of #4372 , it add a procedure that upgrades data from v2 to v3:

  • Moves store_ident_key/prepare_boostrap_key from kv.default to raft.default
  • Moves apply_state_key/region_state_key from kv.raft to raft.default
  • Remove snapshot_raft_state_key

What are the type of the changes? (mandatory)

  • Engineering (engineering change which doesn't change any feature or fix any issue)

How has this PR been tested? (mandatory)

Unit tests and integration tests, however integration tests can not run with out full change of #4372 .

Does this PR affect documentation (docs) update? (mandatory)

No.

Does this PR affect tidb-ansible update? (mandatory)

No.

Refer to a related PR or issue link (optional)

#4372

Signed-off-by: Neil Shen <overvenus@gmail.com>
Signed-off-by: Neil Shen <overvenus@gmail.com>
Signed-off-by: Neil Shen <overvenus@gmail.com>
Signed-off-by: Neil Shen <overvenus@gmail.com>
Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus
Copy link
Member Author

/run-integration-tests

5kbpers
5kbpers previously approved these changes Mar 18, 2019
@5kbpers
Copy link
Member

5kbpers commented Mar 18, 2019

LGTM

@kennytm kennytm added the status/LGT1 Status: PR - There is already 1 approval label Mar 18, 2019
src/raftstore/store/keys.rs Outdated Show resolved Hide resolved
pub fn decode_region_meta_key(key: &[u8]) -> Result<(u64, u8)> {
if REGION_META_PREFIX_KEY.len() + mem::size_of::<u64>() + mem::size_of::<u8>() != key.len() {
/// Decode region key, return the region id and meta suffix type.
fn decode_region_key(prefix: &[u8], key: &[u8], which: &str) -> Result<(u64, u8)> {
Copy link
Member

Choose a reason for hiding this comment

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

You mean category?

src/raftstore/store/peer_storage.rs Show resolved Hide resolved
Signed-off-by: Neil Shen <overvenus@gmail.com>
// Create v2.0.x kv engine.
let kv_cfs_opts = kv_cfg.build_cf_opts_v2();
let mut kv_engine = util::rocksdb_util::new_engine_opt(kv_path, kv_db_opts, kv_cfs_opts)
.unwrap_or_else(|s| panic!("failed to create kv engine: {}", s));
Copy link
Member

Choose a reason for hiding this comment

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

Better return error and let outside to decide to panic or fatal.

pub fn decode_region_meta_key(key: &[u8]) -> Result<(u64, u8)> {
if REGION_META_PREFIX_KEY.len() + mem::size_of::<u64>() + mem::size_of::<u8>() != key.len() {
/// Decode region category key, return the region id and meta suffix type.
fn decode_region_category_key(prefix: &[u8], key: &[u8], which: &str) -> Result<(u64, u8)> {
Copy link
Member

Choose a reason for hiding this comment

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

Oh, what I mean is which is meaningless without context.

Signed-off-by: Neil Shen <overvenus@gmail.com>
Copy link
Member

@BusyJay BusyJay left a comment

Choose a reason for hiding this comment

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

rest LGTM

} else if let Ok((region_id, suffix)) = keys::decode_region_meta_key(key) {
if suffix == keys::REGION_STATE_SUFFIX {
upgrade_raft_wb.put(key, value)?;
info!("upgrading region state"; "region_id" => region_id, "value" => ?value);
Copy link
Member

Choose a reason for hiding this comment

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

Seems too verbose.

Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus overvenus added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels Mar 22, 2019
@overvenus overvenus merged commit c996170 into tikv:master Mar 22, 2019
@overvenus overvenus deleted the upgrade-v2-to-v3 branch March 22, 2019 08:03
sticnarf pushed a commit to sticnarf/tikv that referenced this pull request Oct 27, 2019
Signed-off-by: Neil Shen <overvenus@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 Status: PR - There are already 2 approvals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants