Skip to content

Try to validate read ts for all RPC requests#1513

Merged
ti-chi-bot[bot] merged 11 commits intotikv:masterfrom
MyonKeminta:m/validate-all-read-ts
Dec 9, 2024
Merged

Try to validate read ts for all RPC requests#1513
ti-chi-bot[bot] merged 11 commits intotikv:masterfrom
MyonKeminta:m/validate-all-read-ts

Conversation

@MyonKeminta
Copy link
Contributor

@MyonKeminta MyonKeminta commented Dec 4, 2024

@MyonKeminta
Support validating normal read operations
ea9ca6e 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
pass oracle to RegionRequestSender and do checks for all requests
3ffa5ca 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@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
cfzjywxk
cfzjywxk reviewed Dec 4, 2024
View reviewed changes
@MyonKeminta
Cover write requests that contains implicit read; address comments
5c02543 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Temporarily remove check for write path
e204006 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Also verify ts for tiflash
75cf723 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Add some tests
492cbea 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@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 <MyonKeminta@users.noreply.github.com>
@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

@cfzjywxk cfzjywxk Dec 4, 2024

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

@MyonKeminta MyonKeminta Dec 5, 2024

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
Closed
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
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

Details 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
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
@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 <MyonKeminta@users.noreply.github.com>
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 <MyonKeminta@users.noreply.github.com>
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 <MyonKeminta@users.noreply.github.com>
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 <MyonKeminta@users.noreply.github.com>
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 <MyonKeminta@users.noreply.github.com>
Signed-off-by: you06 <you1474600@gmail.com>

Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com>
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 <MyonKeminta@users.noreply.github.com>
Signed-off-by: you06 <you1474600@gmail.com>

Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com>
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 <MyonKeminta@users.noreply.github.com>
Signed-off-by: you06 <you1474600@gmail.com>

Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com>
This was referenced Jan 14, 2025
Closed
Closed
Closed
Closed
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 <MyonKeminta@users.noreply.github.com>
Signed-off-by: you06 <you1474600@gmail.com>

Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com>
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 <MyonKeminta@users.noreply.github.com>
Signed-off-by: you06 <you1474600@gmail.com>

Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com>
@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 <MyonKeminta@users.noreply.github.com>
Signed-off-by: you06 <you1474600@gmail.com>

Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com>
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 <MyonKeminta@users.noreply.github.com>
Signed-off-by: you06 <you1474600@gmail.com>
Signed-off-by: ekexium <eke@fastmail.com>

Co-authored-by: you06 <you1474600@gmail.com>
Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com>
@ekexium ekexium mentioned this pull request Feb 13, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@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

+1 more reviewer

@crazycs520 crazycs520 crazycs520 approved these changes

Reviewers whose approvals may not affect merge requirements

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