-
Notifications
You must be signed in to change notification settings - Fork 222
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
Let memdb cache last traversed node #1389
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
ekexium
changed the title
Let memdb cache last traversed node
[WIP] Let memdb cache last traversed node
Jul 15, 2024
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
ekexium
changed the title
[WIP] Let memdb cache last traversed node
Let memdb cache last traversed node
Jul 22, 2024
ti-chi-bot
bot
added
the
dco-signoff: yes
Indicates the PR's author has signed the dco.
label
Jul 30, 2024
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
This reverts commit 05fc82b. Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
ekexium
requested review from
cfzjywxk,
you06 and
MyonKeminta
and removed request for
cfzjywxk and
you06
July 31, 2024 14:40
you06
approved these changes
Aug 1, 2024
cfzjywxk
reviewed
Aug 5, 2024
txnkv/transaction/pipelined_flush.go
Outdated
zap.Duration("total_duration", c.txn.GetMemBuffer().GetFlushMetrics().TotalDuration), | ||
zap.Duration("flush_wait_duration", c.txn.GetMemBuffer().GetMetrics().WaitDuration), | ||
zap.Duration("total_duration", c.txn.GetMemBuffer().GetMetrics().TotalDuration), | ||
zap.Uint64("memdb cache hit count", c.txn.GetMemBuffer().GetMetrics().MemDBHitCount), |
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.
The nameing memdb cache hit count
would be confusing for reader who hasn's know the code, perhaps adding traverse
or something like that into the log.
Signed-off-by: ekexium <[email protected]>
cfzjywxk
approved these changes
Aug 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A common usage in TiDB is to access a same key multiple times consecutively. We can save subsequent traversals when this happen.
Benchmark
Sysbench (standard DML)
I tested oltp_write_only, no diff found compared with master.
YCSB benchmark (pipelined DML)
In a YCSB 3M rows benchmark, the computation duration (total duration - flush wait) decreases, proving the optimization works as expected. We also observe a significant increase in flush_wait when applying this optimization. That should be a standalone problem to track and solve, IMO.
Master
insert 47.73-9.74=37.99s
update 30.74-2.78=27.96s
delete 23.81-5.98=17.83s
This PR
insert 43.12-10.50=32.62s
update 31.76-4.96=26.8s
delete 28.82-15.06=13.76s
microbenchmark
master:
This PR: