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

*: add flags to setup backend related config #10283

Merged
merged 1 commit into from
Nov 28, 2018

Conversation

xiang90
Copy link
Contributor

@xiang90 xiang90 commented Nov 26, 2018

The default backend batch interval is 100ms, which is too long for a fast local ssd. It means that a SSD will be almost idle for 100ms for another fdatasync to happen.

Making backend related configuration to be configurable through flags.

@gyuho gyuho requested review from gyuho and jpbetz November 27, 2018 00:00
@xiang90
Copy link
Contributor Author

xiang90 commented Nov 27, 2018

@gyuho

We probably should change the default value from 100ms,10000 to 10ms, 1000 regardless. The current value is too conservative and might hurt latency.

It would be great if you can spend some time to re-do some benchmark with the new value I suggested before changing them.

@gyuho
Copy link
Contributor

gyuho commented Nov 27, 2018

@xiang90 Yeah, I will do it before the v3.4 release!

Copy link
Contributor

@gyuho gyuho left a comment

Choose a reason for hiding this comment

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

lgtm
/cc @jpbetz

@codecov-io
Copy link

Codecov Report

Merging #10283 into master will decrease coverage by 0.28%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10283      +/-   ##
==========================================
- Coverage   71.89%    71.6%   -0.29%     
==========================================
  Files         391      391              
  Lines       36374    36386      +12     
==========================================
- Hits        26150    26055      -95     
- Misses       8420     8521     +101     
- Partials     1804     1810       +6
Impacted Files Coverage Δ
etcdserver/config.go 79.51% <ø> (ø) ⬆️
embed/config.go 56.44% <ø> (ø) ⬆️
etcdserver/backend.go 51.92% <0%> (-9.45%) ⬇️
etcdmain/config.go 83.11% <100%> (+0.15%) ⬆️
embed/etcd.go 70.03% <100%> (+0.11%) ⬆️
proxy/grpcproxy/register.go 69.44% <0%> (-11.12%) ⬇️
pkg/netutil/netutil.go 63.11% <0%> (-7.38%) ⬇️
pkg/tlsutil/tlsutil.go 86.2% <0%> (-6.9%) ⬇️
pkg/fileutil/purge.go 65.9% <0%> (-6.82%) ⬇️
clientv3/leasing/util.go 91.66% <0%> (-6.67%) ⬇️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c649de...3faed21. Read the comment docs.

@jpbetz
Copy link
Contributor

jpbetz commented Nov 27, 2018

lgtm

+1 to decreasing the defaults for 3.4

@xiang90 xiang90 merged commit 829c9b2 into etcd-io:master Nov 28, 2018
@xiang90 xiang90 deleted the more_flags branch November 28, 2018 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants