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.

Sign up for GitHub

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

Jump to bottom

Use fnv.New32a() in hash instead adler32 #40297

Merged
merged 1 commit into from Feb 15, 2017
Merged

Use fnv.New32a() in hash instead adler32 #40297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/controller/lookup_cache.go
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package controller

import (
"hash/adler32"
"hash/fnv"
"sync"

"github.com/golang/groupcache/lru"
Expand All @@ -33,7 +33,7 @@ type objectWithMeta interface {
// Since we match objects by namespace and Labels/Selector, so if two objects have the same namespace and labels,
// they will have the same key.
func keyFunc(obj objectWithMeta) uint64 {
hash := adler32.New()
hash := fnv.New32a()
hashutil.DeepHashObject(hash, &equivalenceLabelObj{
namespace: obj.GetNamespace(),
labels: obj.GetLabels(),
Expand Down
4 changes: 2 additions & 2 deletions pkg/util/mount/mount_linux.go
View file
Open in desktop
Expand Up @@ -21,7 +21,7 @@ package mount
import (
"bufio"
"fmt"
"hash/adler32"
"hash/fnv"
"io"
"os"
"os/exec"
Expand Down Expand Up @@ -282,7 +282,7 @@ func readProcMounts(mountFilePath string, out *[]MountPoint) (uint32, error) {
}

func readProcMountsFrom(file io.Reader, out *[]MountPoint) (uint32, error) {
hash := adler32.New()
hash := fnv.New32a()
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess kubelet is using this utility. Mind explaining what's the impact of this change?

Copy link
Contributor Author

@resouer resouer Feb 6, 2017 •
edited

Choose a reason for hiding this comment

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

This method is used in kubelet to make sure /proc/mounts is steady.

It reads contents of /proc/mounts for multiple times and hash all results to see if any difference happens. There's no cached value of this hash result and it will only be used to compare with the hash result from same method. So I assume it is safe to make this change.

See: https://github.com/kubernetes/kubernetes/blob/master/pkg/util/mount/mount_linux.go#L252-L271

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, thanks!

scanner := bufio.NewReader(file)
for {
line, err := scanner.ReadString('\n')
Expand Down
9 changes: 5 additions & 4 deletions pkg/util/mount/mount_linux_test.go
View file
Open in desktop
Expand Up @@ -29,20 +29,21 @@ func TestReadProcMountsFrom(t *testing.T) {
/dev/1 /path/to/1 type1 flags 1 1
/dev/2 /path/to/2 type2 flags,1,2=3 2 2
`
// NOTE: readProcMountsFrom has been updated to using fnv.New32a()
hash, err := readProcMountsFrom(strings.NewReader(successCase), nil)
if err != nil {
t.Errorf("expected success")
}
if hash != 0xa3522051 {
t.Errorf("expected 0xa3522051, got %#x", hash)
if hash != 0xa290ff0b {
t.Errorf("expected 0xa290ff0b, got %#x", hash)
}
mounts := []MountPoint{}
hash, err = readProcMountsFrom(strings.NewReader(successCase), &mounts)
if err != nil {
t.Errorf("expected success")
}
if hash != 0xa3522051 {
t.Errorf("expected 0xa3522051, got %#x", hash)
if hash != 0xa290ff0b {
t.Errorf("expected 0xa290ff0b, got %#x", hash)
}
if len(mounts) != 3 {
t.Fatalf("expected 3 mounts, got %d", len(mounts))
Expand Down
4 changes: 2 additions & 2 deletions plugin/pkg/scheduler/core/equivalence_cache.go
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package core

import (
"hash/adler32"
"hash/fnv"

"github.com/golang/groupcache/lru"

Expand Down Expand Up @@ -128,7 +128,7 @@ func (ec *EquivalenceCache) SendClearAllCacheReq() {
// hashEquivalencePod returns the hash of equivalence pod.
func (ec *EquivalenceCache) hashEquivalencePod(pod *v1.Pod) uint64 {
equivalencePod := ec.getEquivalencePod(pod)
hash := adler32.New()
hash := fnv.New32a()
hashutil.DeepHashObject(hash, equivalencePod)
return uint64(hash.Sum32())
}