From 036140f305fc34fdefe0ae393b1011f4c3f840de Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Wed, 15 Jul 2015 17:30:03 -0400 Subject: [PATCH] 6221: Fix error reporting. Fix keepstore not to delete blocks with mismatch mtime. Add unit test for BuildTrashLists. --- services/datamanager/datamanager.go | 1 - services/datamanager/keep/keep.go | 8 +-- services/datamanager/summary/pull_list.go | 2 +- .../datamanager/summary/pull_list_test.go | 62 +++++++++---------- services/datamanager/summary/trash_list.go | 46 ++++---------- services/keepstore/trash_worker.go | 1 + 6 files changed, 47 insertions(+), 73 deletions(-) diff --git a/services/datamanager/datamanager.go b/services/datamanager/datamanager.go index 28d558bb8d..d7ac0d2e1f 100644 --- a/services/datamanager/datamanager.go +++ b/services/datamanager/datamanager.go @@ -130,7 +130,6 @@ func singlerun() { replicationSummary.KeepBlocksNotInCollections) summary.WritePullLists(arvLogger, pullLists) - keep.SendTrashLists(arvLogger, kc, trashLists) // Log that we're finished. We force the recording, since go will diff --git a/services/datamanager/keep/keep.go b/services/datamanager/keep/keep.go index 112823ed5f..871acc8e79 100644 --- a/services/datamanager/keep/keep.go +++ b/services/datamanager/keep/keep.go @@ -87,10 +87,10 @@ func init() { // TODO(misha): Change this to include the UUID as well. func (s ServerAddress) String() string { - return s.HostPort() + return s.URL() } -func (s ServerAddress) HostPort() string { +func (s ServerAddress) URL() string { if s.SSL { return fmt.Sprintf("https://%s:%d", s.Host, s.Port) } else { @@ -497,9 +497,7 @@ func SendTrashLists(arvLogger *logger.Logger, kc *keepclient.KeepClient, spl map return } - if resp.StatusCode != http.StatusOK { - log.Printf("Error sending trash list to %v error: %v", url, err.Error()) - } + log.Printf("Sent trash list to %v: response was HTTP %d", url, resp.Status) io.Copy(ioutil.Discard, resp.Body) resp.Body.Close() diff --git a/services/datamanager/summary/pull_list.go b/services/datamanager/summary/pull_list.go index 3a246a2b75..b326c9521a 100644 --- a/services/datamanager/summary/pull_list.go +++ b/services/datamanager/summary/pull_list.go @@ -94,7 +94,7 @@ func ComputePullServers(kc *keepclient.KeepClient, serverHasBlock := map[string]struct{}{} for _, info := range serversStoringBlock { sa := keepServerInfo.KeepServerIndexToAddress[info.ServerIndex] - serverHasBlock[cs.Get(sa.HostPort())] = struct{}{} + serverHasBlock[cs.Get(sa.URL())] = struct{}{} } roots := keepclient.NewRootSorter(kc.LocalRoots(), diff --git a/services/datamanager/summary/pull_list_test.go b/services/datamanager/summary/pull_list_test.go index fb8631b1fe..f22d47d29b 100644 --- a/services/datamanager/summary/pull_list_test.go +++ b/services/datamanager/summary/pull_list_test.go @@ -54,7 +54,7 @@ func (s *MySuite) TestCreatePullServers(c *C) { c.Check( CreatePullServers(cs, - stringSet("keep0:25107", "keep1:25108"), + stringSet("https://keep0:25107", "https://keep1:25108"), stringSet(), []string{}, 5), @@ -63,64 +63,64 @@ func (s *MySuite) TestCreatePullServers(c *C) { c.Check( CreatePullServers(cs, - stringSet("keep0:25107", "keep1:25108"), - stringSet("keep0:25107"), - []string{"keep0:25107"}, + stringSet("https://keep0:25107", "https://keep1:25108"), + stringSet("https://keep0:25107"), + []string{"https://keep0:25107"}, 5), DeepEquals, - PullServers{To: []string{}, From: []string{"keep0:25107"}}) + PullServers{To: []string{}, From: []string{"https://keep0:25107"}}) c.Check( CreatePullServers(cs, - stringSet("keep0:25107", "keep1:25108"), - stringSet("keep3:25110", "keep2:25109", "keep1:25108", "keep0:25107"), - []string{"keep3:25110", "keep2:25109", "keep1:25108", "keep0:25107"}, + stringSet("https://keep0:25107", "https://keep1:25108"), + stringSet("https://keep3:25110", "https://keep2:25109", "https://keep1:25108", "https://keep0:25107"), + []string{"https://keep3:25110", "https://keep2:25109", "https://keep1:25108", "https://keep0:25107"}, 5), DeepEquals, - PullServers{To: []string{"keep3:25110", "keep2:25109"}, - From: []string{"keep1:25108", "keep0:25107"}}) + PullServers{To: []string{"https://keep3:25110", "https://keep2:25109"}, + From: []string{"https://keep1:25108", "https://keep0:25107"}}) c.Check( CreatePullServers(cs, - stringSet("keep0:25107", "keep1:25108"), - stringSet("keep3:25110", "keep1:25108", "keep0:25107"), - []string{"keep3:25110", "keep2:25109", "keep1:25108", "keep0:25107"}, + stringSet("https://keep0:25107", "https://keep1:25108"), + stringSet("https://keep3:25110", "https://keep1:25108", "https://keep0:25107"), + []string{"https://keep3:25110", "https://keep2:25109", "https://keep1:25108", "https://keep0:25107"}, 5), DeepEquals, - PullServers{To: []string{"keep3:25110"}, - From: []string{"keep1:25108", "keep0:25107"}}) + PullServers{To: []string{"https://keep3:25110"}, + From: []string{"https://keep1:25108", "https://keep0:25107"}}) c.Check( CreatePullServers(cs, - stringSet("keep0:25107", "keep1:25108"), - stringSet("keep3:25110", "keep2:25109", "keep1:25108", "keep0:25107"), - []string{"keep3:25110", "keep2:25109", "keep1:25108", "keep0:25107"}, + stringSet("https://keep0:25107", "https://keep1:25108"), + stringSet("https://keep3:25110", "https://keep2:25109", "https://keep1:25108", "https://keep0:25107"), + []string{"https://keep3:25110", "https://keep2:25109", "https://keep1:25108", "https://keep0:25107"}, 1), DeepEquals, - PullServers{To: []string{"keep3:25110"}, - From: []string{"keep1:25108", "keep0:25107"}}) + PullServers{To: []string{"https://keep3:25110"}, + From: []string{"https://keep1:25108", "https://keep0:25107"}}) c.Check( CreatePullServers(cs, - stringSet("keep0:25107", "keep1:25108"), - stringSet("https://keep3:25110", "http://keep2:25109", - "https://keep1:25108", "http://keep0:25107"), - []string{"https://keep3:25110", "http://keep2:25109", - "https://keep1:25108", "http://keep0:25107"}, + stringSet("https://keep0:25107", "https://keep1:25108"), + stringSet("https://keep3:25110", "https://keep2:25109", + "https://keep1:25108", "https://keep0:25107"), + []string{"https://keep3:25110", "https://keep2:25109", + "https://keep1:25108", "https://keep0:25107"}, 1), DeepEquals, - PullServers{To: []string{"keep3:25110"}, - From: []string{"https://keep1:25108", "http://keep0:25107"}}) + PullServers{To: []string{"https://keep3:25110"}, + From: []string{"https://keep1:25108", "https://keep0:25107"}}) c.Check( CreatePullServers(cs, - stringSet("keep0:25107", "keep1:25108"), - stringSet("keep3:25110", "keep2:25109", "keep1:25108", "keep0:25107"), - []string{"keep3:25110", "keep2:25109", "keep1:25108", "keep0:25107"}, + stringSet("https://keep0:25107", "https://keep1:25108"), + stringSet("https://keep3:25110", "https://keep2:25109", "https://keep1:25108", "https://keep0:25107"), + []string{"https://keep3:25110", "https://keep2:25109", "https://keep1:25108", "https://keep0:25107"}, 0), DeepEquals, PullServers{To: []string{}, - From: []string{"keep1:25108", "keep0:25107"}}) + From: []string{"https://keep1:25108", "https://keep0:25107"}}) } // Checks whether two pull list maps are equal. Since pull lists are diff --git a/services/datamanager/summary/trash_list.go b/services/datamanager/summary/trash_list.go index efb40e25a7..330cd4e45a 100644 --- a/services/datamanager/summary/trash_list.go +++ b/services/datamanager/summary/trash_list.go @@ -2,15 +2,9 @@ package summary import ( - "encoding/json" - "fmt" "git.curoverse.com/arvados.git/sdk/go/keepclient" - "git.curoverse.com/arvados.git/sdk/go/logger" "git.curoverse.com/arvados.git/services/datamanager/keep" - "git.curoverse.com/arvados.git/services/datamanager/loggerutil" "log" - "os" - "strings" "time" ) @@ -24,12 +18,10 @@ func BuildTrashLists(kc *keepclient.KeepClient, writableServers[url] = struct{}{} } - m = make(map[string]keep.TrashList) - _ttl, err := kc.Arvados.Discovery("blobSignatureTtl") if err != nil { log.Printf("Failed to get blobSignatureTtl: %v", err) - return + return map[string]keep.TrashList{} } ttl := int64(_ttl.(float64)) @@ -37,6 +29,16 @@ func BuildTrashLists(kc *keepclient.KeepClient, // expire unreferenced blocks more than "ttl" seconds old. expiry := time.Now().UTC().Unix() - ttl + return BuildTrashListsInternal(writableServers, keepServerInfo, expiry, keepBlocksNotInCollections) +} + +func BuildTrashListsInternal(writableServers map[string]struct{}, + keepServerInfo *keep.ReadServers, + expiry int64, + keepBlocksNotInCollections BlockSet) (m map[string]keep.TrashList) { + + m = make(map[string]keep.TrashList) + for block, _ := range keepBlocksNotInCollections { for _, block_on_server := range keepServerInfo.BlockToServers[block] { if block_on_server.Mtime < expiry { @@ -52,31 +54,5 @@ func BuildTrashLists(kc *keepclient.KeepClient, } } return -} -// Writes each pull list to a file. -// The filename is based on the hostname. -// -// This is just a hack for prototyping, it is not expected to be used -// in production. -func WriteTrashLists(arvLogger *logger.Logger, - trashLists map[string]keep.TrashList) { - r := strings.NewReplacer(":", ".") - for host, list := range trashLists { - filename := fmt.Sprintf("trash_list.%s", r.Replace(RemoveProtocolPrefix(host))) - trashListFile, err := os.Create(filename) - if err != nil { - loggerutil.FatalWithMessage(arvLogger, - fmt.Sprintf("Failed to open %s: %v", filename, err)) - } - defer trashListFile.Close() - - enc := json.NewEncoder(trashListFile) - err = enc.Encode(list) - if err != nil { - loggerutil.FatalWithMessage(arvLogger, - fmt.Sprintf("Failed to write trash list to %s: %v", filename, err)) - } - log.Printf("Wrote trash list to %s.", filename) - } } diff --git a/services/keepstore/trash_worker.go b/services/keepstore/trash_worker.go index 5e56f030e2..69fbf7456a 100644 --- a/services/keepstore/trash_worker.go +++ b/services/keepstore/trash_worker.go @@ -42,6 +42,7 @@ func TrashItem(trashRequest TrashRequest) { } if trashRequest.BlockMtime != mtime.Unix() { log.Printf("%v Delete(%v): mtime does not match", volume, trashRequest.Locator) + continue } if never_delete { -- 2.30.2