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.

Sign up for GitHub

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

Jump to bottom

Try to validate read ts for all RPC requests #1513

Merged
merged 11 commits into from
Dec 9, 2024
Merged

Try to validate read ts for all RPC requests #1513

merged 11 commits into from
Dec 9, 2024

Conversation

MyonKeminta
Copy link
Contributor

@ti-chi-bot ti-chi-bot bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has signed the dco. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 4, 2024
@ti-chi-bot ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 4, 2024
@MyonKeminta
Add tests
1806599 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta MyonKeminta marked this pull request as ready for review December 4, 2024 09:57
@ti-chi-bot ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 4, 2024
cfzjywxk
cfzjywxk reviewed Dec 4, 2024
View reviewed changes
Copy link
Contributor

@cfzjywxk cfzjywxk 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

oracle/oracles/pd.go
return nil
}

latestTSInfo, exists := o.getLastTSWithArrivalTS(opt.TxnScope)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove the txnScope related check and map structure within this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to be removed when all code related to txn scope is being removed, to keep the code consistent.

cfzjywxk
cfzjywxk approved these changes Dec 5, 2024
View reviewed changes
Copy link
Contributor

@cfzjywxk cfzjywxk 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-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Dec 5, 2024
@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Dec 5, 2024
@cfzjywxk cfzjywxk mentioned this pull request Dec 5, 2024
Open
7 tasks
@cfzjywxk
Copy link
Contributor

cfzjywxk commented Dec 6, 2024

/retest

@MyonKeminta MyonKeminta mentioned this pull request Dec 6, 2024
Merged
13 tasks
@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 9, 2024
@ti-chi-bot ti-chi-bot bot added the lgtm label Dec 9, 2024
@ti-chi-bot ti-chi-bot
Copy link

ti-chi-bot bot commented Dec 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, crazycs520, zyguan

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:
  • OWNERS [cfzjywxk,crazycs520,zyguan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 9, 2024
@ti-chi-bot ti-chi-bot
Copy link

ti-chi-bot bot commented Dec 9, 2024

[LGTM Timeline notifier]

Timeline:

  • 2024-12-05 07:40:42.772255757 +0000 UTC m=+1313430.391910273: ☑️ agreed by cfzjywxk.
  • 2024-12-05 07:49:57.251986982 +0000 UTC m=+1313984.871641495: ☑️ agreed by zyguan.
  • 2024-12-06 08:57:11.403377726 +0000 UTC m=+1404419.023032242: ✖️🔁 reset by MyonKeminta.
  • 2024-12-06 09:20:18.83825978 +0000 UTC m=+1405806.457914295: ☑️ agreed by cfzjywxk.
  • 2024-12-06 09:29:04.79328888 +0000 UTC m=+1406332.412943396: ☑️ agreed by crazycs520.
  • 2024-12-09 06:06:57.745843367 +0000 UTC m=+245807.834645911: ✖️🔁 reset by MyonKeminta.
  • 2024-12-09 07:30:59.296741888 +0000 UTC m=+250849.385544431: ☑️ agreed by cfzjywxk.
  • 2024-12-09 09:47:07.068586765 +0000 UTC m=+259017.157389305: ☑️ agreed by crazycs520.

@ti-chi-bot ti-chi-bot bot merged commit 06d7f4b into tikv:master Dec 9, 2024
12 checks passed
@MyonKeminta MyonKeminta deleted the m/validate-all-read-ts branch December 10, 2024 04:14
you06-pingcap pushed a commit to you06/client-go that referenced this pull request Jan 7, 2025
@MyonKeminta @you06
Try to validate read ts for all RPC requests (tikv#1513)
fa9def6 
Signed-off-by: MyonKeminta <[email protected]>
you06-pingcap pushed a commit to you06/client-go that referenced this pull request Jan 7, 2025
@MyonKeminta @you06
Try to validate read ts for all RPC requests (tikv#1513)
ac320ef 
Signed-off-by: MyonKeminta <[email protected]>
you06-pingcap pushed a commit to you06/client-go that referenced this pull request Jan 7, 2025
@MyonKeminta @you06
Try to validate read ts for all RPC requests (tikv#1513)
924a048 
Signed-off-by: MyonKeminta <[email protected]>
you06-pingcap pushed a commit to you06/client-go that referenced this pull request Jan 7, 2025
@MyonKeminta @you06
Try to validate read ts for all RPC requests (tikv#1513)
1b01708 
Signed-off-by: MyonKeminta <[email protected]>
This was referenced Jan 7, 2025
Merged
Merged
Merged
Merged
ti-chi-bot bot pushed a commit that referenced this pull request Jan 8, 2025
@you06 @MyonKeminta
Try to validate read ts for all RPC requests (#1513) (#1546)
7b4409c 
 

Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: you06 <[email protected]>

Co-authored-by: MyonKeminta <[email protected]>
ti-chi-bot bot pushed a commit that referenced this pull request Jan 13, 2025
@you06 @MyonKeminta
Try to validate read ts for all RPC requests (#1513) (#1549)
d66e460 
 

Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: you06 <[email protected]>

Co-authored-by: MyonKeminta <[email protected]>
ti-chi-bot bot pushed a commit that referenced this pull request Jan 13, 2025
@you06 @MyonKeminta
Try to validate read ts for all RPC requests (#1513) (#1547)
8691f3d 
 

Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: you06 <[email protected]>

Co-authored-by: MyonKeminta <[email protected]>
This was referenced Jan 14, 2025
Open
Open
Open
Open
Closed
ti-chi-bot bot pushed a commit that referenced this pull request Jan 15, 2025
@you06 @MyonKeminta
Try to validate read ts for all RPC requests (#1513) (#1548)
c1b98e6 
 

Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: you06 <[email protected]>

Co-authored-by: MyonKeminta <[email protected]>
ekexium pushed a commit to ekexium/client-go that referenced this pull request Jan 15, 2025
@you06 @MyonKeminta @ekexium
Try to validate read ts for all RPC requests (tikv#1513) (tikv#1546)
3de2311 
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: you06 <[email protected]>

Co-authored-by: MyonKeminta <[email protected]>
@ekexium ekexium mentioned this pull request Jan 15, 2025
Merged
ekexium pushed a commit to ekexium/client-go that referenced this pull request Jan 15, 2025
@you06 @MyonKeminta @ekexium
Try to validate read ts for all RPC requests (tikv#1513) (tikv#1546)
d0b21cf 
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: you06 <[email protected]>

Co-authored-by: MyonKeminta <[email protected]>
ti-chi-bot bot pushed a commit that referenced this pull request Jan 16, 2025
@ekexium @you06 @MyonKeminta
Try to validate read ts for all RPC requests (#1513) (#1558)
ec354dc 
 

Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: you06 <[email protected]>
Signed-off-by: ekexium <[email protected]>

Co-authored-by: you06 <[email protected]>
Co-authored-by: MyonKeminta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crazycs520 crazycs520 crazycs520 approved these changes

@cfzjywxk cfzjywxk cfzjywxk approved these changes

@you06 you06 Awaiting requested review from you06

@ekexium ekexium Awaiting requested review from ekexium

@zyguan zyguan Awaiting requested review from zyguan

Assignees
No one assigned
Labels
approved dco-signoff: yes Indicates the PR's author has signed the dco. lgtm size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MyonKeminta @cfzjywxk @zyguan @crazycs520