etcdserver: use same ReadView for read-only txns #8071

heyitsanthony · 2017-06-09T03:25:12Z

Fixes inconsistencies from writes interfering with read-only txns.

codecov-io · 2017-06-09T13:32:43Z

Codecov Report

Merging #8071 into master will decrease coverage by 0.14%.
The diff coverage is 91.89%.

@@            Coverage Diff             @@
##           master    #8071      +/-   ##
==========================================
- Coverage   76.66%   76.51%   -0.15%     
==========================================
  Files         341      341              
  Lines       26511    26511              
==========================================
- Hits        20324    20286      -38     
- Misses       4741     4773      +32     
- Partials     1446     1452       +6

Impacted Files	Coverage Δ
mvcc/kv.go	`40% <100%> (+40%)`	⬆️
etcdserver/apply.go	`88.82% <91.42%> (-0.22%)`	⬇️
mvcc/kv_view.go	`80% <0%> (-20%)`	⬇️
clientv3/namespace/watch.go	`87.87% <0%> (-12.13%)`	⬇️
pkg/fileutil/purge.go	`73.68% <0%> (-7.9%)`	⬇️
pkg/adt/interval_tree.go	`89.63% <0%> (-3.66%)`	⬇️
raft/node.go	`90.17% <0%> (-2.24%)`	⬇️
rafthttp/msgappv2_codec.go	`71.3% <0%> (-1.74%)`	⬇️
rafthttp/peer.go	`90.22% <0%> (-1.51%)`	⬇️
etcdserver/cluster_util.go	`76.64% <0%> (-1.46%)`	⬇️
... and 8 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 a8c073c...a47cafd. Read the comment docs.

gyuho

lgtm. thanks!
Will backport to 3.2 after merge.

/cc @xiang90

Already used internally by mvcc, but needed by etcdserver txns.

A read-only txn isn't serialized by raft, but it uses a fresh read txn for every mvcc access prior to executing its request ops. If a write txn modifies the keys matching the read txn's comparisons, the read txn may return inconsistent results. To fix, use the same read-only mvcc txn for the duration of the etcd txn. Probably gets a modest txn speedup as well since there are fewer read txn allocations.

heyitsanthony added this to the v3.2.0 milestone Jun 9, 2017

heyitsanthony force-pushed the txn-rev branch from 1487fee to a47cafd Compare June 9, 2017 04:46

gyuho approved these changes Jun 9, 2017

View reviewed changes

Anthony Romano added 3 commits June 9, 2017 09:20

integration: test txn comparison and concurrent put ordering

ad22aaa

mvcc: create TxnWrites from TxnRead with NewReadOnlyTxnWrite

da48f1f

Already used internally by mvcc, but needed by etcdserver txns.

heyitsanthony force-pushed the txn-rev branch from a47cafd to d173b09 Compare June 9, 2017 16:20

heyitsanthony merged commit 56841bb into etcd-io:master Jun 9, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

etcdserver: use same ReadView for read-only txns #8071

etcdserver: use same ReadView for read-only txns #8071

heyitsanthony commented Jun 9, 2017 •

edited

Loading

codecov-io commented Jun 9, 2017

gyuho left a comment

etcdserver: use same ReadView for read-only txns #8071

etcdserver: use same ReadView for read-only txns #8071

Conversation

heyitsanthony commented Jun 9, 2017 • edited Loading

codecov-io commented Jun 9, 2017

Codecov Report

gyuho left a comment

Choose a reason for hiding this comment

heyitsanthony commented Jun 9, 2017 •

edited

Loading