-
Notifications
You must be signed in to change notification settings - Fork 205
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
fix: bug in distributed wal(obkv, kafka) #422
fix: bug in distributed wal(obkv, kafka) #422
Conversation
Please describe the solution in |
I don't think the current tests are sufficient to cover this bug. |
The amount of code modification is large. Please tell me where is the core modified code. |
272ee34
to
1a0c6aa
Compare
Done. |
Done. |
The codes in |
… version -> region version`.
1a0c6aa
to
13440c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* change `shard_id` in `TableData` to `shard_info` * modify wal to adapt adding shard version. * add test for makeing wal perceive shard version. * add more logs in key path to help debug. * change the mapping from `shard version -> region version` to `cluster version -> region version`. * address CR.
Which issue does this PR close?
Closes #441
Rationale for this change
If a shard (which is mapped to region in wal) is move between nodes like this:
Then wal module in mode A can't distinguish if the shard has been moved.
This may cause a serious bug:
What changes are included in this PR?
1. For rocksdb and obkv impls
before:
now:
Before,
Table unit
inwal manager
is only indexed bytable id
, sowal manager
know nothing aboutshard id
andshard version
(shard are mapped to region). So whereshard id
andshard version
oftable
changed,wal manager
can't perceive.Now,
Table unit
is indexed byregion version
+region id
+table id
, it can perceive any changes.2. For kafka impl
before:
now:
Similar as above,
region
in kafka impl can perceiveshard version
now.Are there any user-facing changes?
None.
How does this change test
Test by ut.