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.

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

Fake PR just for review #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fake PR just for review #4

wants to merge 1 commit into from

Conversation

innerr
Copy link
Owner

@innerr innerr commented May 22, 2020

Signed-off-by: Liu Cong [email protected]

What problem does this PR solve?

Improve TiKV performance on non-NVMe disk.

Problem Summary:

  • The 'sync' performance is varied on different types of disk, it could from several hundred to 50K times per sec, we could use fio -fdatasync or pg_test_sync to profile the 'sync' performance of a disk
  • Typically, a non-NVMe disk could perform 2K~5K sync per sec
  • When TiKV runs under proper stress (lots of write, disk latency is good), the 'sync' frequency could reach 20K as we discovered on grafana panel

So, the 'sync' performance will be one of the bottlenecks of TiKV performance, especially under some types of disk.
This PR provide a config entry to control the raftstore's 'sync' frequency.

What is changed and how it works?

When peers receive data, the 'write' operation goes as usual, but the 'sync' will be hold for a while.
(not holding 'write' operation is the key, IO will be smoother comparing to simple batching, which will hold both 'write' and 'sync')

Before 'sync' is called, the related raft follower's messages of AppendResp will be blocked(held and cached),
when follower perform 'sync', the raft log will be considered well persisted, then the messages will be released and delivered to leader.

In current implementation of raft-rs, the committed index of a raft group will advance immediately, even though the log is not persisted yet, because currently it assume that the log will(should, must) be persisted before next round of handling.
This assumption is not fit for our purpose, so we changed raft-rs for a bit:

Related changes

Tests

  • Integration test
  • Manual test (add detailed scripts or steps below)

Side effects

  • These changes could delay the committed logs' applying, and cause greater latency, so we provide a config entry to enable/disable it when running TiKV on different types of disk.

Release note

  • Control sync frequency using config entry: raftstore.delay-sync-ns, performance on some types of disk could be better with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant