6221: Fix error reporting. Fix keepstore not to delete blocks with mismatch

mtime.  Add unit test for BuildTrashLists.

diff --git a/services/datamanager/datamanager.go b/services/datamanager/datamanager.go
index 28d558bb8d28711563e72eafb37f2aadb00371d0..d7ac0d2e1fb415bc6e563d6bc744d681aa2717ec 100644 (file)
--- 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 112823ed5fed036b3d29a257086dd41dc93b5582..871acc8e793929b909b1256b3ff7e14a90059eba 100644 (file)
--- 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 3a246a2b757339e9a8cc8a50076ba116f9049db8..b326c9521ab0d7b545fd52c340c2b17455ea5aa5 100644 (file)
--- 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 fb8631b1feb39df3580b1b70da69d4c3f1e5d160..f22d47d29b1e4560b054d70d1ecd845c8b38fdb4 100644 (file)
--- 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 efb40e25a7d20e0ef84563d2367b96c7d7bfdd7c..330cd4e45a33a23ed01a9946c10c76dcbea34d7e 100644 (file)
--- 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 5e56f030e28c7616d876a80838869e45b2dc10b7..69fbf7456a12f2bb1a39cae0e7c9760f0a12d102 100644 (file)
--- 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 {