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

clientv3: fix sync base #5883

Merged
merged 2 commits into from
Jul 7, 2016
Merged

clientv3: fix sync base #5883

merged 2 commits into from
Jul 7, 2016

Conversation

westhood
Copy link
Contributor

@westhood westhood commented Jul 6, 2016

It is not correct to use WithPrefix. Range end will change in every
batch.

@xiang90
Copy link
Contributor

xiang90 commented Jul 6, 2016

Range end will change in every batch.

I am not sure I can understand this. Why should the prefix change per batch?

ctx := context.TODO()

for i := 0; i < 2000; i++ {
if _, err := cli.Put(ctx, fmt.Sprintf("test%d", i), "test"); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

this test does not work without this patch?

ctx := context.TODO()

for i := 0; i < 2000; i++ {
if _, err := cli.Put(ctx, fmt.Sprintf("test%d", i), "test"); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

probably we need to spin up a bunch of go-routines (50?) to finish the put. Sync put would be slow, each would require a fsync (>10ms on hdd).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok,I will fix it。

It is not correct to use WithPrefix. Range end will change in every
internal batch.
@xiang90
Copy link
Contributor

xiang90 commented Jul 7, 2016

@westhood Thanks. lgtm.

@xiang90 xiang90 merged commit b6a4972 into etcd-io:master Jul 7, 2016
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.

None yet

4 participants