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

etcd-tester: add txn stresser #8510

Merged
merged 2 commits into from
Jan 9, 2018
Merged

etcd-tester: add txn stresser #8510

merged 2 commits into from
Jan 9, 2018

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Sep 6, 2017

functional-tester never failed for the last several months.
This adds Txn stresser.
Address #8293.

@gyuho gyuho requested a review from heyitsanthony September 6, 2017 21:18
@gyuho gyuho added the WIP label Sep 7, 2017
@gyuho gyuho removed the WIP label Sep 7, 2017
@gyuho gyuho force-pushed the txn-stresser branch 2 times, most recently from 00488c3 to da510d7 Compare September 8, 2017 16:51

etcdRunnerPath: *etcdRunnerPath,
}
if scfg.keyTxnSuffixRange == 0 || scfg.keyTxnSuffixRange > 100 {
Copy link
Contributor

Choose a reason for hiding this comment

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

0 should mean don't use the txn stresser

}
}

return func(ctx context.Context) (error, int64) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is too big to inline, use a separate function:

return func(ctx context.Context) { return writeTxn(ctx, kvc, keys) }

}

return func(ctx context.Context) (error, int64) {
n := rand.Intn(keyTxnSuffixRange)
Copy link
Contributor

Choose a reason for hiding this comment

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

The number of keys selected shouldn't be coupled to the number of keys in the working set. Half the time it'll be checking the state of half the keys or more and only one key has to be present to delete everything. Probably should set it to some small number of ops like n := rand.Intn(64) and the user can configure the conflict rate by adjusting the txn suffix range.

@heyitsanthony
Copy link
Contributor

Ideally the flat txn tester would be the nested txn tester with depth set to 1.

@gyuho gyuho added the WIP label Sep 8, 2017
@gyuho gyuho force-pushed the txn-stresser branch 3 times, most recently from eb68578 to 3a9a734 Compare September 11, 2017 20:04
@gyuho gyuho added WIP and removed WIP labels Sep 11, 2017
@gyuho gyuho removed the WIP label Sep 11, 2017
@xiang90 xiang90 added this to the v3.4.0 milestone Sep 18, 2017
@@ -67,16 +69,22 @@ func (s *keyStresser) Stress() error {
kvc := pb.NewKVClient(conn)

var stressEntries = []stressEntry{
{weight: 0.7, f: newStressPut(kvc, s.keySuffixRange, s.keySize)},
{weight: 0.24, f: newStressPut(kvc, s.keySuffixRange, s.keySize)},
Copy link
Member

Choose a reason for hiding this comment

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

why .24?

}

// add nested txns if any
for i := 1; i < depth; i++ {
Copy link
Member

Choose a reason for hiding this comment

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

I dont think i understand the depth here.

it seems to me that len(txn.Success) or len(txn.Failure) == depth. e.g
txnReq.Success[nested, nested, nested]

I thought depth means
txnReq.Success[nested[nested[nested]]]

maybe i am missing something.

func getTxnReqs(key, val string) (com *pb.Compare, delOp *pb.RequestOp, putOp *pb.RequestOp) {
com = &pb.Compare{
Key: []byte(key),
Target: pb.Compare_VERSION,
Copy link
Member

Choose a reason for hiding this comment

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

is version key version?

If version of key > 0 => key exists and then delete the key?
if version of key == 0 => no key, so put key there?

Copy link
Member

@fanminshi fanminshi Jan 9, 2018

Choose a reason for hiding this comment

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

also in what case will the version of key == 0?

from line 219 kvc.Put(context.Background(), &pb.PutRequest{, all the key are written to etcd already.

@gyuho gyuho force-pushed the txn-stresser branch 2 times, most recently from df69011 to 81b4675 Compare January 9, 2018 21:38
@@ -47,6 +47,8 @@ func main() {
stressKeyLargeSize := flag.Uint("stress-key-large-size", 32*1024+1, "the size of each large key written into etcd.")
stressKeySize := flag.Uint("stress-key-size", 100, "the size of each small key written into etcd.")
stressKeySuffixRange := flag.Uint("stress-key-count", 250000, "the count of key range written into etcd.")
stressKeyTxnSuffixRange := flag.Uint("stress-key-txn-count", 100, "the count of key range written into etcd txn (max 100).")
stressKeyTxnOps := flag.Uint("stress-key-txn-ops", 1, "ops of transaction stressers (max 64).")
Copy link
Member

@fanminshi fanminshi Jan 9, 2018

Choose a reason for hiding this comment

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

ops of transaction stressers (max 64) -> number of operations per transaction (max 64)?

I am unsure what does transaction stressers means.

plog.Fatalf("stress-key-txn-count is maximum 100, got %d", scfg.keyTxnSuffixRange)
}
if scfg.keyTxnOps > 64 {
plog.Fatalf("stress-key-txn-depth is maximum 64, got %d", scfg.keyTxnOps)
Copy link
Member

Choose a reason for hiding this comment

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

s/depth/ops

@gyuho
Copy link
Contributor Author

gyuho commented Jan 9, 2018

@fanminshi All addressed.

@fanminshi
Copy link
Member

lgtm

@gyuho gyuho merged commit 52f73c5 into etcd-io:master Jan 9, 2018
@gyuho gyuho deleted the txn-stresser branch January 9, 2018 22:17
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