From 5d118141cd3d92f7d501b7045992df9e9f312b8d Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 21 Jul 2016 19:07:00 -0700 Subject: [PATCH] XL: Remove deadcode unionChecksumInfo. (#2261) --- globals.go | 7 ++--- host-to-ip.go | 5 ---- xl-v1-utils.go | 62 +++++++++------------------------------ xl-v1-utils_test.go | 71 ++++++++++++++++++++------------------------- xl-v1_test.go | 28 ------------------ 5 files changed, 47 insertions(+), 126 deletions(-) diff --git a/globals.go b/globals.go index 51a11e10c..3cc5aec27 100644 --- a/globals.go +++ b/globals.go @@ -34,7 +34,6 @@ const ( globalMinioCertFile = "public.crt" globalMinioKeyFile = "private.key" globalMinioConfigFile = "config.json" - globalMinioProfilePath = "profile" // Add new global values here. ) @@ -55,8 +54,6 @@ var ( // global colors. var ( - colorWhite = color.New(color.FgWhite).SprintfFunc() - colorWhiteBold = color.New(color.FgWhite, color.Bold).SprintfFunc() - colorBlue = color.New(color.FgBlue).SprintfFunc() - colorBold = color.New(color.Bold).SprintFunc() + colorBlue = color.New(color.FgBlue).SprintfFunc() + colorBold = color.New(color.Bold).SprintFunc() ) diff --git a/host-to-ip.go b/host-to-ip.go index 7cda67c9e..fc1352543 100644 --- a/host-to-ip.go +++ b/host-to-ip.go @@ -40,8 +40,3 @@ func getIPsFromHosts(hosts []string) (ips []net.IP) { sort.Sort(sort.Reverse(byLastOctet(ips))) return ips } - -// getHostToIP - parses a host string into net.IP value. -func getHostToIP(host string) net.IP { - return net.ParseIP(host) -} diff --git a/xl-v1-utils.go b/xl-v1-utils.go index 98ecefa9e..a8e4f3f20 100644 --- a/xl-v1-utils.go +++ b/xl-v1-utils.go @@ -18,7 +18,6 @@ package main import ( "encoding/json" - "errors" "hash/crc32" "path" ) @@ -76,17 +75,22 @@ func diskCount(disks []StorageAPI) int { return diskCount } -// hashOrder - returns consistent hashed integers of count slice, based on the input token. -func hashOrder(token string, count int) []int { - if count < 0 { - panic(errors.New("hashOrder count cannot be negative")) +// hashOrder - hashes input key to return returns consistent +// hashed integer slice. Returned integer order is salted +// with an input key. This results in consistent order. +// NOTE: collisions are fine, we are not looking for uniqueness +// in the slices returned. +func hashOrder(key string, cardinality int) []int { + if cardinality < 0 { + // Returns an empty int slice for negative cardinality. + return nil } - nums := make([]int, count) - tokenCrc := crc32.Checksum([]byte(token), crc32.IEEETable) + nums := make([]int, cardinality) + keyCrc := crc32.Checksum([]byte(key), crc32.IEEETable) - start := int(uint32(tokenCrc)%uint32(count)) | 1 - for i := 1; i <= count; i++ { - nums[i-1] = 1 + ((start + i) % count) + start := int(uint32(keyCrc)%uint32(cardinality)) | 1 + for i := 1; i <= cardinality; i++ { + nums[i-1] = 1 + ((start + i) % cardinality) } return nums } @@ -108,44 +112,6 @@ func readXLMeta(disk StorageAPI, bucket string, object string) (xlMeta xlMetaV1, return xlMeta, nil } -// Uses a map to find union of checksums of parts that were concurrently written -// but committed before this part. N B For a different, concurrent upload of -// the same part, the ongoing request's data/metadata prevails. -// cur - corresponds to parts written to disk before the ongoing putObjectPart request -// updated - corresponds to parts written to disk while the ongoing putObjectPart is in progress -// curPartName - name of the part that is being written -// returns []checkSumInfo containing the set union of checksums of parts that -// have been written so far incl. the part being written. -func unionChecksumInfos(cur []checkSumInfo, updated []checkSumInfo, curPartName string) []checkSumInfo { - checksumSet := make(map[string]checkSumInfo) - var checksums []checkSumInfo - - checksums = cur - for _, cksum := range checksums { - checksumSet[cksum.Name] = cksum - } - - checksums = updated - for _, cksum := range checksums { - // skip updating checksum of the part that is - // written in this request because the checksum - // from cur, corresponding to this part, - // should remain. - if cksum.Name == curPartName { - continue - } - checksumSet[cksum.Name] = cksum - } - - // Form the checksumInfo to be committed in xl.json - // from the map. - var finalChecksums []checkSumInfo - for _, cksum := range checksumSet { - finalChecksums = append(finalChecksums, cksum) - } - return finalChecksums -} - // Return ordered partsMetadata depeinding on distribution. func getOrderedPartsMetadata(distribution []int, partsMetadata []xlMetaV1) (orderedPartsMetadata []xlMetaV1) { orderedPartsMetadata = make([]xlMetaV1, len(partsMetadata)) diff --git a/xl-v1-utils_test.go b/xl-v1-utils_test.go index c82eae058..1f905a6db 100644 --- a/xl-v1-utils_test.go +++ b/xl-v1-utils_test.go @@ -16,9 +16,13 @@ package main -import "testing" +import ( + "reflect" + "testing" +) -// Test for reduceErrs. +// Test for reduceErrs, reduceErr reduces collection +// of errors into a single maximal error with in the list. func TestReduceErrs(t *testing.T) { // List all of all test cases to validate various cases of reduce errors. testCases := []struct { @@ -57,48 +61,35 @@ func TestReduceErrs(t *testing.T) { } } -// Test for unionChecksums -func TestUnionChecksumInfos(t *testing.T) { - cur := []checkSumInfo{ - {"part.1", "dummy", "cur-hash.1"}, - {"part.2", "dummy", "cur-hash.2"}, - {"part.3", "dummy", "cur-hash.3"}, - {"part.4", "dummy", "cur-hash.4"}, - {"part.5", "dummy", "cur-hash.5"}, +// TestHashOrder - test order of ints in array +func TestHashOrder(t *testing.T) { + testCases := []struct { + objectName string + hashedOrder []int + }{ + // cases which should pass the test. + // passing in valid object name. + {"object", []int{15, 16, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}}, + {"The Shining Script .pdf", []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}}, + {"Cost Benefit Analysis (2009-2010).pptx", []int{15, 16, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}}, + {"117Gn8rfHL2ACARPAhaFd0AGzic9pUbIA/5OCn5A", []int{3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1, 2}}, + {"SHØRT", []int{11, 12, 13, 14, 15, 16, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}, + {"There are far too many object names, and far too few bucket names!", []int{15, 16, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}}, + {"a/b/c/", []int{3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1, 2}}, + {"/a/b/c", []int{7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1, 2, 3, 4, 5, 6}}, + {string([]byte{0xff, 0xfe, 0xfd}), []int{15, 16, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}}, } - updated := []checkSumInfo{ - {"part.1", "dummy", "updated-hash.1"}, - {"part.2", "dummy", "updated-hash.2"}, - {"part.3", "dummy", "updated-hash.3"}, - } - curPartcksum := cur[0] // part.1 is the current part being written - // Verify that hash of current part being written must be from cur []checkSumInfo - finalChecksums := unionChecksumInfos(cur, updated, curPartcksum.Name) - for _, cksum := range finalChecksums { - if cksum.Name == curPartcksum.Name && cksum.Hash != curPartcksum.Hash { - t.Errorf("expected Hash = %s but received Hash = %s\n", curPartcksum.Hash, cksum.Hash) + // Tests hashing order to be consistent. + for i, testCase := range testCases { + hashedOrder := hashOrder(testCase.objectName, 16) + if !reflect.DeepEqual(testCase.hashedOrder, hashedOrder) { + t.Errorf("Test case %d: Expected \"%#v\" but failed \"%#v\"", i+1, testCase.hashedOrder, hashedOrder) } } - // Verify that all part checksums are present in the union and nothing more. - // Map to store all unique part names - allPartNames := make(map[string]struct{}) - // Insert part names from cur and updated []checkSumInfo - for _, cksum := range cur { - allPartNames[cksum.Name] = struct{}{} - } - for _, cksum := range updated { - allPartNames[cksum.Name] = struct{}{} - } - // All parts must have an entry in the []checkSumInfo returned from unionChecksums - for _, finalcksum := range finalChecksums { - if _, ok := allPartNames[finalcksum.Name]; !ok { - t.Errorf("expected to find %s but not present in the union, where current part is %s\n", - finalcksum.Name, curPartcksum.Name) - } - } - if len(finalChecksums) != len(allPartNames) { - t.Error("Union of Checksums doesn't have same number of elements as unique parts in total") + // Tests hashing order to fail for when order is '-1'. + if hashedOrder := hashOrder("This will fail", -1); hashedOrder != nil { + t.Errorf("Test: Expect \"nil\" but failed \"%#v\"", hashedOrder) } } diff --git a/xl-v1_test.go b/xl-v1_test.go index a781be780..b56d581e0 100644 --- a/xl-v1_test.go +++ b/xl-v1_test.go @@ -19,7 +19,6 @@ package main import ( "os" "path/filepath" - "reflect" "testing" ) @@ -137,30 +136,3 @@ func TestNewXL(t *testing.T) { t.Fatalf("Unable to initialize erasure, %s", err) } } - -// TestHashOrder - test order of ints in array -func TestHashOrder(t *testing.T) { - testCases := []struct { - objectName string - hashedOrder []int - }{ - // cases which should pass the test. - // passing in valid object name. - {"object", []int{15, 16, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}}, - {"The Shining Script .pdf", []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}}, - {"Cost Benefit Analysis (2009-2010).pptx", []int{15, 16, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}}, - {"117Gn8rfHL2ACARPAhaFd0AGzic9pUbIA/5OCn5A", []int{3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1, 2}}, - {"SHØRT", []int{11, 12, 13, 14, 15, 16, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}, - {"There are far too many object names, and far too few bucket names!", []int{15, 16, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}}, - {"a/b/c/", []int{3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1, 2}}, - {"/a/b/c", []int{7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1, 2, 3, 4, 5, 6}}, - {string([]byte{0xff, 0xfe, 0xfd}), []int{15, 16, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}}, - } - - for i, testCase := range testCases { - hashedOrder := hashOrder(testCase.objectName, 16) - if !reflect.DeepEqual(testCase.hashedOrder, hashedOrder) { - t.Errorf("Test case %d: Expected \"%#v\" but failed \"%#v\"", i+1, testCase.hashedOrder, hashedOrder) - } - } -}