support write proxy data to uni ps#6848
Conversation
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. DetailsReviewer can indicate their review by submitting an approval review. |
d1cdd24 to
a756b5b
Compare
a756b5b to
871089a
Compare
dde459d to
3d91ea6
Compare
|
/run-all-tests |
| // For example, suppose a key in tikv to be {0x01, 0x02, 0x03}. | ||
| // If it is written by raft engine, then actual key uni ps see is the same as in tikv. | ||
| // But if it is written by kv engine, the actual key uni ps see will be {0x01, 0x01, 0x02, 0x03}. |
There was a problem hiding this comment.
Will this cause problems with scanning raft keys? If raft-engine scan with '\x01' prefix, it will unexpectedly see some keys actually written to the kv engine.
There was a problem hiding this comment.
what about this?
- prepend another '\x01' for raft engine keys
- prepend '\x02' for kv engine keys
There was a problem hiding this comment.
Prepend will cause a memory allocation. And there will be many raft log written to raft engine. So I suggest not change the format of raft engine key.
But we should prepend '\x02' instead of '\0x01` for kv engine keys to avoid they scan keys written by other one.
There was a problem hiding this comment.
Then we should let the tikv's raft-engine project be aware of '\x02' prefix is reserved, and not to use this prefix in the future. /cc @CalvinNeo
There was a problem hiding this comment.
This should not be difficult. The convention that all raft keys in tikv start with 0x01 won't easily be changed.
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| option(ENABLE_UNI_PS_FFI "Enable write proxy data to uni ps" OFF) |
There was a problem hiding this comment.
Is the behavior "write proxy data to uni ps" controlled by this compile config but not a runtime config?
There was a problem hiding this comment.
Yes, but change it to a runtime config requires non-trivial change in proxy part. We may do it in another pr.
JaySon-Huang
left a comment
There was a problem hiding this comment.
Are my comments correct?
dbms/src/Storages/Page/V3/Universal/UniversalPageIdFormatImpl.h
Outdated
Show resolved
Hide resolved
dbms/src/Storages/Page/V3/Universal/UniversalPageIdFormatImpl.h
Outdated
Show resolved
Hide resolved
dbms/src/Storages/Page/V3/Universal/UniversalPageIdFormatImpl.h
Outdated
Show resolved
Hide resolved
Co-authored-by: JaySon <tshent@qq.com>
Co-authored-by: JaySon <tshent@qq.com>
Co-authored-by: JaySon <tshent@qq.com>
Good catch. Almost correct. |
|
/merge |
|
@CalvinNeo: It seems you want to merge this PR, I will help you trigger all the tests: /run-all-tests You only need to trigger If you have any questions about the PR merge process, please refer to pr process. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
|
This pull request has been accepted and is ready to merge. DetailsCommit hash: ef80fd9 |
What problem does this PR solve?
Issue Number: close #6728
Problem Summary: need support write proxy data to the global uni ps instance.
What is changed and how it works?
Add some ffi to support write proxy data to uni ps, mainly including write, delete, read, scan.
Check List
Tests
Side effects
Documentation
Release note