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

mvcc: Optimize compaction for short commit pauses #11034

Merged
merged 4 commits into from
Aug 15, 2019

Conversation

jpbetz
Copy link
Contributor

@jpbetz jpbetz commented Aug 14, 2019

Reduce compaction intervals to 1k and duration between compactions to 10ms.

Also forces immediate commit of the write buffer after each compaction (thanks @jingyih for this).

I'll also make this configurable via a flag.

Supersedes #11021

@codecov-io
Copy link

codecov-io commented Aug 14, 2019

Codecov Report

Merging #11034 into master will increase coverage by 0.59%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11034      +/-   ##
==========================================
+ Coverage   63.67%   64.27%   +0.59%     
==========================================
  Files         402      402              
  Lines       37719    37720       +1     
==========================================
+ Hits        24018    24244     +226     
+ Misses      12079    11841     -238     
- Partials     1622     1635      +13
Impacted Files Coverage Δ
mvcc/kvstore_compaction.go 90.47% <100%> (+12.42%) ⬆️
client/keys.go 55.77% <0%> (-27.64%) ⬇️
pkg/transport/timeout_conn.go 60% <0%> (-20%) ⬇️
proxy/grpcproxy/register.go 69.44% <0%> (-11.12%) ⬇️
clientv3/watch.go 90.94% <0%> (-1.06%) ⬇️
clientv3/leasing/kv.go 89.7% <0%> (-0.67%) ⬇️
mvcc/watchable_store.go 83.5% <0%> (ø) ⬆️
etcdserver/cluster_util.go 56.91% <0%> (+0.39%) ⬆️
etcdserver/v3_server.go 70.38% <0%> (+0.45%) ⬆️
etcdserver/server.go 68.59% <0%> (+0.46%) ⬆️
... and 14 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 fd0d43d...821e98c. Read the comment docs.

@gyuho
Copy link
Contributor

gyuho commented Aug 15, 2019

@jpbetz @jingyih Can we get this in by Friday, so we have at least 2 weeks to test this before the release :)

Also, I still don't see the documentation about ForceCommit calls. We should document it is to force commit the write buffer after each compaction to make sure desired compaction is persisted (is there any other reason that I am missing?).

@jpbetz
Copy link
Contributor Author

jpbetz commented Aug 15, 2019

@gyuho Sorry for the delay. The tests around compaction are fidgety and I'm still working on getting them updated. I'll try to get that done today. I must have lost the ForceCommit comment when reverting some local changes, I'll get that re-added.

@jpbetz
Copy link
Contributor Author

jpbetz commented Aug 15, 2019

Confirmed flag and ENV_VAR are working as expected. Tests are passing locally.

@gyuho gyuho changed the title [WIP] mvcc: Optimize compaction for short commit pauses mvcc: Optimize compaction for short commit pauses Aug 15, 2019
@gyuho
Copy link
Contributor

gyuho commented Aug 15, 2019

Hmm... 32-bit e2e tests are failing... Debugging at #11041.

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>
@gyuho
Copy link
Contributor

gyuho commented Aug 15, 2019

Found the issue

../../bin/etcd-12491: panic: runtime error: invalid memory address or nil pointer dereference
../../bin/etcd-12491: [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x804a40b]
../../bin/etcd-12491: 
../../bin/etcd-12491: goroutine 1 [running]:
../../bin/etcd-12491: runtime/internal/atomic.Load64(0xa0cc014, 0x85a92cc, 0x85a92cc)
../../bin/etcd-12491: 	/usr/local/go/src/runtime/internal/atomic/asm_386.s:194 +0xb
../../bin/etcd-12491: go.etcd.io/etcd/mvcc.(*store).ConsistentIndex(0xa0cc000, 0x0, 0x0)
../../bin/etcd-12491: 	/go/src/go.etcd.io/etcd/mvcc/kvstore.go:567 +0x3e
../../bin/etcd-12491: go.etcd.io/etcd/etcdserver.NewServer(0xffe60cfe, 0x9, 0x0, 0x0, 0x0, 0x0, 0xa1d99c0, 0x1, 0x1, 0xa1d9b40, ...)
../../bin/etcd-12491: 	/go/src/go.etcd.io/etcd/etcdserver/server.go:570 +0x1b9c
../../bin/etcd-12491: go.etcd.io/etcd/embed.StartEtcd(0xa0e8f00, 0xa1dab00, 0x0, 0x0)
../../bin/etcd-12491: 	/go/src/go.etcd.io/etcd/embed/etcd.go:211 +0x86f
../../bin/etcd-12491: go.etcd.io/etcd/etcdmain.startEtcd(0xa0e8f00, 0x8a971ec, 0x3, 0x8a98201, 0x5)
../../bin/etcd-12491: 	/go/src/go.etcd.io/etcd/etcdmain/etcd.go:302 +0x25
../../bin/etcd-12491: go.etcd.io/etcd/etcdmain.startEtcdOrProxyV2()
../../bin/etcd-12491: 	/go/src/go.etcd.io/etcd/etcdmain/etcd.go:160 +0x2437
../../bin/etcd-12491: go.etcd.io/etcd/etcdmain.Main()
../../bin/etcd-12491: 	/go/src/go.etcd.io/etcd/etcdmain/main.go:46 +0x32
../../bin/etcd-12491: main.main()
../../bin/etcd-12491: 	/go/src/go.etcd.io/etcd/main.go:28 +0x17

@gyuho gyuho merged commit c55410c into etcd-io:master Aug 15, 2019
jingyih added a commit to jingyih/website that referenced this pull request Sep 3, 2019
Remove mentioning of using concurrent read in compaction. The original PR etcd-io/etcd#11021 was superseded by etcd-io/etcd#11034, which no long uses concurrent read in compaction.
jingyih added a commit to jingyih/website that referenced this pull request Sep 13, 2019
Remove mentioning of using concurrent read in compaction. The original PR etcd-io/etcd#11021 was superseded by etcd-io/etcd#11034, which no long uses concurrent read in compaction.
k8s-ci-robot pushed a commit to kubernetes/website that referenced this pull request Sep 13, 2019
Remove mentioning of using concurrent read in compaction. The original PR etcd-io/etcd#11021 was superseded by etcd-io/etcd#11034, which no long uses concurrent read in compaction.
it2911 pushed a commit to it2911/kubernetes-website that referenced this pull request Sep 17, 2019
Remove mentioning of using concurrent read in compaction. The original PR etcd-io/etcd#11021 was superseded by etcd-io/etcd#11034, which no long uses concurrent read in compaction.
wahyuoi pushed a commit to wahyuoi/website that referenced this pull request Sep 18, 2019
Remove mentioning of using concurrent read in compaction. The original PR etcd-io/etcd#11021 was superseded by etcd-io/etcd#11034, which no long uses concurrent read in compaction.
for _, key := range keys {
rev = bytesToRev(key)
if _, ok := keep[rev]; !ok {
tx.UnsafeDelete(keyBucketName, key)
keyCompactions++
Copy link
Contributor

Choose a reason for hiding this comment

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

why we deleting this

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

Successfully merging this pull request may close these issues.

4 participants