Merge branch 'master' into 7167-keep-rsync
[arvados.git] / services / datamanager / summary / pull_list.go
index 542a6693d2358535dfb4e1b8d76c910af853cd02..cc01249a624a7f4947cdcfc8dafd73dd7e347377 100644 (file)
@@ -1,4 +1,5 @@
 // Code for generating pull lists as described in https://arvados.org/projects/arvados/wiki/Keep_Design_Doc#Pull-List
+
 package summary
 
 import (
@@ -14,19 +15,21 @@ import (
        "strings"
 )
 
+// Locator is a block digest
 type Locator blockdigest.DigestWithSize
 
+// MarshalJSON encoding
 func (l Locator) MarshalJSON() ([]byte, error) {
        return []byte("\"" + blockdigest.DigestWithSize(l).String() + "\""), nil
 }
 
-// One entry in the Pull List
+// PullRequest represents one entry in the Pull List
 type PullRequest struct {
        Locator Locator  `json:"locator"`
        Servers []string `json:"servers"`
 }
 
-// The Pull List for a particular server
+// PullList for a particular server
 type PullList []PullRequest
 
 // PullListByLocator implements sort.Interface for PullList based on
@@ -49,6 +52,7 @@ func (a PullListByLocator) Less(i, j int) bool {
        return false
 }
 
+// PullServers struct
 // For a given under-replicated block, this structure represents which
 // servers should pull the specified block and which servers they can
 // pull it from.
@@ -57,8 +61,12 @@ type PullServers struct {
        From []string // Servers that already contain the specified block
 }
 
-// Creates a map from block locator to PullServers with one entry for
-// each under-replicated block.
+// ComputePullServers creates a map from block locator to PullServers
+// with one entry for each under-replicated block.
+//
+// This method ignores zero-replica blocks since there are no servers
+// to pull them from, so callers should feel free to omit them, but
+// this function will ignore them if they are provided.
 func ComputePullServers(kc *keepclient.KeepClient,
        keepServerInfo *keep.ReadServers,
        blockToDesiredReplication map[blockdigest.DigestWithSize]int,
@@ -71,27 +79,26 @@ func ComputePullServers(kc *keepclient.KeepClient,
        // Servers that are writeable
        writableServers := map[string]struct{}{}
        for _, url := range kc.WritableLocalRoots() {
-               writableServers[cs.Get(RemoveProtocolPrefix(url))] = struct{}{}
+               writableServers[cs.Get(url)] = struct{}{}
        }
 
-       for block, _ := range underReplicated {
+       for block := range underReplicated {
                serversStoringBlock := keepServerInfo.BlockToServers[block]
                numCopies := len(serversStoringBlock)
                numCopiesMissing := blockToDesiredReplication[block] - numCopies
                if numCopiesMissing > 0 {
                        // We expect this to always be true, since the block was listed
                        // in underReplicated.
-                       // TODO(misha): Consider asserting the above conditional.
 
                        if numCopies > 0 {
-                               // I believe that we should expect this to always be true.
+                               // Not much we can do with blocks with no copies.
 
                                // A server's host-port string appears as a key in this map
                                // iff it contains the block.
                                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(),
@@ -106,9 +113,9 @@ func ComputePullServers(kc *keepclient.KeepClient,
        return m
 }
 
-// Creates a pull list in which the To and From fields preserve the
-// ordering of sorted servers and the contents are all canonical
-// strings.
+// CreatePullServers creates a pull list in which the To and From
+// fields preserve the ordering of sorted servers and the contents
+// are all canonical strings.
 func CreatePullServers(cs CanonicalString,
        serverHasBlock map[string]struct{},
        writableServers map[string]struct{},
@@ -123,13 +130,13 @@ func CreatePullServers(cs CanonicalString,
        for _, host := range sortedServers {
                // Strip the protocol portion of the url.
                // Use the canonical copy of the string to avoid memory waste.
-               server := cs.Get(RemoveProtocolPrefix(host))
+               server := cs.Get(host)
                _, hasBlock := serverHasBlock[server]
                if hasBlock {
                        // The from field should include the protocol.
                        ps.From = append(ps.From, cs.Get(host))
                } else if len(ps.To) < maxToFields {
-                       _, writable := writableServers[server]
+                       _, writable := writableServers[host]
                        if writable {
                                ps.To = append(ps.To, server)
                        }
@@ -139,12 +146,12 @@ func CreatePullServers(cs CanonicalString,
        return
 }
 
-// Strips the protocol prefix from a url.
+// RemoveProtocolPrefix strips the protocol prefix from a url.
 func RemoveProtocolPrefix(url string) string {
        return url[(strings.LastIndex(url, "/") + 1):]
 }
 
-// Produces a PullList for each keep server.
+// BuildPullLists produces a PullList for each keep server.
 func BuildPullLists(lps map[Locator]PullServers) (spl map[string]PullList) {
        spl = map[string]PullList{}
        // We don't worry about canonicalizing our strings here, because we
@@ -163,7 +170,7 @@ func BuildPullLists(lps map[Locator]PullServers) (spl map[string]PullList) {
        return
 }
 
-// Writes each pull list to a file.
+// WritePullLists 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
@@ -172,7 +179,7 @@ func WritePullLists(arvLogger *logger.Logger,
        pullLists map[string]PullList) {
        r := strings.NewReplacer(":", ".")
        for host, list := range pullLists {
-               filename := fmt.Sprintf("pull_list.%s", r.Replace(host))
+               filename := fmt.Sprintf("pull_list.%s", r.Replace(RemoveProtocolPrefix(host)))
                pullListFile, err := os.Create(filename)
                if err != nil {
                        loggerutil.FatalWithMessage(arvLogger,