-
Notifications
You must be signed in to change notification settings - Fork 39
enhance: Support specify mvccTimestamp for probe pk
#377
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
enhance: Support specify mvccTimestamp for probe pk
#377
Conversation
Add `--mvccTimestamp` param for probe pk command Signed-off-by: Congqi Xia <[email protected]>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: congqixia The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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.
Pull Request Overview
This PR enhances the probe pk command by allowing users to specify an MVCC timestamp via the new --mvccTimestamp parameter. Key changes include:
- Removal of the commented-out probe PK subcommand from the command list.
- Addition of the MvccTimestamp field in the ProbePKParam struct.
- Conditional logic to default to the current timestamp if no MVCC timestamp is provided, and updating the request using the provided timestamp.
@@ -249,6 +248,11 @@ func (s *InstanceState) ProbePKCommand(ctx context.Context, p *ProbePKParam) err | |||
return nil | |||
} | |||
|
|||
// use current ts when mvccTimestamp not specified | |||
if p.MvccTimestamp == 0 { | |||
p.MvccTimestamp = int64(ComposeTS(time.Now().UnixMilli(), 0)) |
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.
Consider using a consistent type for MvccTimestamp. The ProbePKParam struct defines it as int64, yet later it is converted to uint64 when constructing the request. Updating the field's type to uint64 could help avoid potential conversion issues.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
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.
Param type not support uint64 yet. will fix this when framework parameter type supports more data type
/lgtm |
Add
--mvccTimestamp
param for probe pk command