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

storage: fix hash by iterating kv #3585

Merged
merged 1 commit into from
Sep 23, 2015
Merged

storage: fix hash by iterating kv #3585

merged 1 commit into from
Sep 23, 2015

Conversation

xiang90
Copy link
Contributor

@xiang90 xiang90 commented Sep 23, 2015

child buckets are stored in a map in-memory during a transaction and iterated over when flushed. there’s no ordering guarantee of maps in Go. so the snapshot data might be different even if the kv are identical.

@xiang90
Copy link
Contributor Author

xiang90 commented Sep 23, 2015

/cc @gyuho

@gyuho
Copy link
Contributor

gyuho commented Sep 23, 2015

Ah! That's what was happening.
It's really good to learn. Thanks for
keeping me updated on this!

b := tx.Bucket(next)
if b == nil {
return fmt.Errorf("cannot get hash of bucket %s", string(next))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

could we write the name of bucket into crc32 too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@gyuho gyuho mentioned this pull request Sep 23, 2015
@yichengq
Copy link
Contributor

👍 Amazing to find this out.

@yichengq
Copy link
Contributor

LGTM after ci pass

xiang90 added a commit that referenced this pull request Sep 23, 2015
storage: fix hash by iterating kv
@xiang90 xiang90 merged commit 0813a0f into etcd-io:master Sep 23, 2015
@xiang90 xiang90 deleted the fix_hash branch September 23, 2015 18:39
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.

3 participants