Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

fill size in SSTMeta (#233) #702

Merged
merged 1 commit into from
Jan 19, 2021

Conversation

ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Jan 18, 2021

cherry-pick #233 to release-4.0
You can switch your code base to this Pull Request by using git-extras:

# In br repo:
git pr https://github.com/pingcap/br/pull/702

After apply modifications, you can push your change to this PR via:

git push git@github.com:ti-srebot/br.git pr/702:release-4.0-634701b9f73e

What problem does this PR solve?

Fix #163

What is changed and how it works?

fill size in sstmeta. but we need make sure this change work on tikv which has this
tikv/tikv#7256, otherwise it will panic tikv.

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch

Release note

  • Fix the issue that missing fileSize in SSTMeta might cause TiKV to generate a big region.

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented Jan 18, 2021

@3pointer
Copy link
Collaborator

/run-all-tests

@3pointer
Copy link
Collaborator

/rebuild

@overvenus
Copy link
Member

Please add a version check, otherwise it may cause TiKV panic.

Copy link
Collaborator

@3pointer 3pointer left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 LGTM1 label Jan 18, 2021
@3pointer
Copy link
Collaborator

Please add a version check, otherwise it may cause TiKV panic.

we had this check already, but forgot to pick #233 to release-4.0

@overvenus
Copy link
Member

LGTM

Please also add a test in another PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants