From d5142a69848df7fa506b8cb16a76cb621598769a Mon Sep 17 00:00:00 2001 From: Radhika Chippada Date: Mon, 18 May 2015 23:11:59 -0400 Subject: [PATCH] 4717: rename writableRoots as writableLocalRoots. --- sdk/go/keepclient/keepclient.go | 23 ++--- sdk/go/keepclient/keepclient_test.go | 84 +++++++++---------- sdk/go/keepclient/support.go | 8 +- sdk/python/arvados/keep.py | 7 +- ...150512193020_read_only_on_keep_services.rb | 6 +- services/keepproxy/keepproxy_test.go | 4 +- services/keepstore/pull_worker.go | 2 +- 7 files changed, 61 insertions(+), 73 deletions(-) diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go index 5ebdf0ad7a..f82e5c7c59 100644 --- a/sdk/go/keepclient/keepclient.go +++ b/sdk/go/keepclient/keepclient.go @@ -38,8 +38,8 @@ type KeepClient struct { Want_replicas int Using_proxy bool localRoots *map[string]string + writableLocalRoots *map[string]string gatewayRoots *map[string]string - writableRoots *map[string]string lock sync.RWMutex Client *http.Client } @@ -195,12 +195,12 @@ func (kc *KeepClient) GatewayRoots() map[string]string { return *kc.gatewayRoots } -// WritableRoots() returns the map of writable Keep services: +// WritableLocalRoots() returns the map of writable local Keep services: // uuid -> baseURI. -func (kc *KeepClient) WritableRoots() map[string]string { +func (kc *KeepClient) WritableLocalRoots() map[string]string { kc.lock.RLock() defer kc.lock.RUnlock() - return *kc.writableRoots + return *kc.writableLocalRoots } // SetServiceRoots updates the localRoots and gatewayRoots maps, @@ -210,24 +210,27 @@ func (kc *KeepClient) WritableRoots() map[string]string { // caller can reuse/modify them after SetServiceRoots returns, but // they should not be modified by any other goroutine while // SetServiceRoots is running. -func (kc *KeepClient) SetServiceRoots(newLocals, newGateways map[string]string, writableRoots map[string]string) { +func (kc *KeepClient) SetServiceRoots(newLocals, newWritableLocals map[string]string, newGateways map[string]string) { locals := make(map[string]string) for uuid, root := range newLocals { locals[uuid] = root } + + writables := make(map[string]string) + for uuid, root := range newWritableLocals { + writables[uuid] = root + } + gateways := make(map[string]string) for uuid, root := range newGateways { gateways[uuid] = root } - writables := make(map[string]string) - for uuid, root := range writableRoots { - writables[uuid] = root - } + kc.lock.Lock() defer kc.lock.Unlock() kc.localRoots = &locals + kc.writableLocalRoots = &writables kc.gatewayRoots = &gateways - kc.writableRoots = &writables } // getSortedRoots returns a list of base URIs of Keep services, in the diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go index e22ffe06f0..c1f6a3e6f9 100644 --- a/sdk/go/keepclient/keepclient_test.go +++ b/sdk/go/keepclient/keepclient_test.go @@ -243,17 +243,17 @@ func (s *StandaloneSuite) TestPutB(c *C) { kc.Want_replicas = 2 arv.ApiToken = "abc123" localRoots := make(map[string]string) - writableRoots := make(map[string]string) + writableLocalRoots := make(map[string]string) ks := RunSomeFakeKeepServers(st, 5) for i, k := range ks { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url defer k.listener.Close() } - kc.SetServiceRoots(localRoots, nil, writableRoots) + kc.SetServiceRoots(localRoots, writableLocalRoots, nil) kc.PutB([]byte("foo")) @@ -288,17 +288,17 @@ func (s *StandaloneSuite) TestPutHR(c *C) { kc.Want_replicas = 2 arv.ApiToken = "abc123" localRoots := make(map[string]string) - writableRoots := make(map[string]string) + writableLocalRoots := make(map[string]string) ks := RunSomeFakeKeepServers(st, 5) for i, k := range ks { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url defer k.listener.Close() } - kc.SetServiceRoots(localRoots, nil, writableRoots) + kc.SetServiceRoots(localRoots, writableLocalRoots, nil) reader, writer := io.Pipe() @@ -344,23 +344,23 @@ func (s *StandaloneSuite) TestPutWithFail(c *C) { kc.Want_replicas = 2 arv.ApiToken = "abc123" localRoots := make(map[string]string) - writableRoots := make(map[string]string) + writableLocalRoots := make(map[string]string) ks1 := RunSomeFakeKeepServers(st, 4) ks2 := RunSomeFakeKeepServers(fh, 1) for i, k := range ks1 { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url defer k.listener.Close() } for i, k := range ks2 { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i+len(ks1))] = k.url - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i+len(ks1))] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i+len(ks1))] = k.url defer k.listener.Close() } - kc.SetServiceRoots(localRoots, nil, writableRoots) + kc.SetServiceRoots(localRoots, writableLocalRoots, nil) shuff := NewRootSorter( kc.LocalRoots(), Md5String("foo")).GetSortedRoots() @@ -403,23 +403,23 @@ func (s *StandaloneSuite) TestPutWithTooManyFail(c *C) { kc.Want_replicas = 2 arv.ApiToken = "abc123" localRoots := make(map[string]string) - writableRoots := make(map[string]string) + writableLocalRoots := make(map[string]string) ks1 := RunSomeFakeKeepServers(st, 1) ks2 := RunSomeFakeKeepServers(fh, 4) for i, k := range ks1 { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url defer k.listener.Close() } for i, k := range ks2 { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i+len(ks1))] = k.url - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i+len(ks1))] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i+len(ks1))] = k.url defer k.listener.Close() } - kc.SetServiceRoots(localRoots, nil, writableRoots) + kc.SetServiceRoots(localRoots, writableLocalRoots, nil) _, replicas, err := kc.PutB([]byte("foo")) @@ -464,7 +464,7 @@ func (s *StandaloneSuite) TestGet(c *C) { arv, err := arvadosclient.MakeArvadosClient() kc, _ := MakeKeepClient(&arv) arv.ApiToken = "abc123" - kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, map[string]string{ks.url: ""}) + kc.SetServiceRoots(map[string]string{"x": ks.url}, map[string]string{ks.url: ""}, nil) r, n, url2, err := kc.Get(hash) defer r.Close() @@ -490,7 +490,7 @@ func (s *StandaloneSuite) TestGetFail(c *C) { arv, err := arvadosclient.MakeArvadosClient() kc, _ := MakeKeepClient(&arv) arv.ApiToken = "abc123" - kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, map[string]string{ks.url: ""}) + kc.SetServiceRoots(map[string]string{"x": ks.url}, map[string]string{ks.url: ""}, nil) r, n, url2, err := kc.Get(hash) c.Check(err, Equals, BlockNotFound) @@ -525,8 +525,8 @@ func (s *StandaloneSuite) TestGetWithServiceHint(c *C) { arv.ApiToken = "abc123" kc.SetServiceRoots( map[string]string{"x": ks0.url}, - map[string]string{uuid: ks.url}, - map[string]string{ks0.url: "", ks.url: ""}) + map[string]string{"x": ks0.url}, + map[string]string{uuid: ks.url}) r, n, uri, err := kc.Get(hash + "+K@" + uuid) defer r.Close() @@ -619,8 +619,8 @@ func (s *StandaloneSuite) TestGetWithServiceHintFailoverToLocals(c *C) { arv.ApiToken = "abc123" kc.SetServiceRoots( map[string]string{"zzzzz-bi6l4-keepdisk0000000": ksLocal.url}, - map[string]string{uuid: ksGateway.url}, - map[string]string{"zzzzz-bi6l4-keepdisk0000000": ksLocal.url}) + map[string]string{"zzzzz-bi6l4-keepdisk0000000": ksLocal.url}, + map[string]string{uuid: ksGateway.url}) r, n, uri, err := kc.Get(hash + "+K@" + uuid) c.Assert(err, Equals, nil) @@ -654,7 +654,7 @@ func (s *StandaloneSuite) TestChecksum(c *C) { arv, err := arvadosclient.MakeArvadosClient() kc, _ := MakeKeepClient(&arv) arv.ApiToken = "abc123" - kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, map[string]string{ks.url: ""}) + kc.SetServiceRoots(map[string]string{"x": ks.url}, map[string]string{ks.url: ""}, nil) r, n, _, err := kc.Get(barhash) _, err = ioutil.ReadAll(r) @@ -689,23 +689,23 @@ func (s *StandaloneSuite) TestGetWithFailures(c *C) { kc, _ := MakeKeepClient(&arv) arv.ApiToken = "abc123" localRoots := make(map[string]string) - writableRoots := make(map[string]string) + writableLocalRoots := make(map[string]string) ks1 := RunSomeFakeKeepServers(st, 1) ks2 := RunSomeFakeKeepServers(fh, 4) for i, k := range ks1 { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url defer k.listener.Close() } for i, k := range ks2 { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i+len(ks1))] = k.url - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i+len(ks1))] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i+len(ks1))] = k.url defer k.listener.Close() } - kc.SetServiceRoots(localRoots, nil, writableRoots) + kc.SetServiceRoots(localRoots, writableLocalRoots, nil) // This test works only if one of the failing services is // attempted before the succeeding service. Otherwise, @@ -786,17 +786,17 @@ func (s *StandaloneSuite) TestPutProxy(c *C) { kc.Using_proxy = true arv.ApiToken = "abc123" localRoots := make(map[string]string) - writableRoots := make(map[string]string) + writableLocalRoots := make(map[string]string) ks1 := RunSomeFakeKeepServers(st, 1) for i, k := range ks1 { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url defer k.listener.Close() } - kc.SetServiceRoots(localRoots, nil, writableRoots) + kc.SetServiceRoots(localRoots, writableLocalRoots, nil) _, replicas, err := kc.PutB([]byte("foo")) <-st.handled @@ -819,16 +819,16 @@ func (s *StandaloneSuite) TestPutProxyInsufficientReplicas(c *C) { kc.Using_proxy = true arv.ApiToken = "abc123" localRoots := make(map[string]string) - writableRoots := make(map[string]string) + writableLocalRoots := make(map[string]string) ks1 := RunSomeFakeKeepServers(st, 1) for i, k := range ks1 { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url defer k.listener.Close() } - kc.SetServiceRoots(localRoots, nil, writableRoots) + kc.SetServiceRoots(localRoots, writableLocalRoots, nil) _, replicas, err := kc.PutB([]byte("foo")) <-st.handled @@ -878,9 +878,7 @@ func (s *StandaloneSuite) TestMakeLocatorInvalidInput(c *C) { c.Check(err, Equals, InvalidLocatorError) } -func (s *StandaloneSuite) TestPutBWant2ReplicasWithOnlyOneWritableRoots(c *C) { - log.Printf("TestPutWant2ReplicasWithOnlyOneWritableRoots") - +func (s *StandaloneSuite) TestPutBWant2ReplicasWithOnlyOneWritableLocalRoot(c *C) { hash := Md5String("foo") st := StubPutHandler{ @@ -896,19 +894,19 @@ func (s *StandaloneSuite) TestPutBWant2ReplicasWithOnlyOneWritableRoots(c *C) { kc.Want_replicas = 2 arv.ApiToken = "abc123" localRoots := make(map[string]string) - writableRoots := make(map[string]string) + writableLocalRoots := make(map[string]string) ks := RunSomeFakeKeepServers(st, 5) for i, k := range ks { localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url if i == 0 { - writableRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url + writableLocalRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", i)] = k.url } defer k.listener.Close() } - kc.SetServiceRoots(localRoots, nil, writableRoots) + kc.SetServiceRoots(localRoots, writableLocalRoots, nil) _, replicas, err := kc.PutB([]byte("foo")) @@ -916,13 +914,9 @@ func (s *StandaloneSuite) TestPutBWant2ReplicasWithOnlyOneWritableRoots(c *C) { c.Check(replicas, Equals, 1) c.Check(<-st.handled, Equals, localRoots[fmt.Sprintf("zzzzz-bi6l4-fakefakefake%03d", 0)]) - - log.Printf("TestPutWant2ReplicasWithOnlyOneWritableRoots done") } -func (s *StandaloneSuite) TestPutBWithNoWritableRoots(c *C) { - log.Printf("TestPutBWithNoWritableRoots") - +func (s *StandaloneSuite) TestPutBWithNoWritableLocalRoots(c *C) { hash := Md5String("foo") st := StubPutHandler{ @@ -938,7 +932,7 @@ func (s *StandaloneSuite) TestPutBWithNoWritableRoots(c *C) { kc.Want_replicas = 2 arv.ApiToken = "abc123" localRoots := make(map[string]string) - writableRoots := make(map[string]string) + writableLocalRoots := make(map[string]string) ks := RunSomeFakeKeepServers(st, 5) @@ -947,12 +941,10 @@ func (s *StandaloneSuite) TestPutBWithNoWritableRoots(c *C) { defer k.listener.Close() } - kc.SetServiceRoots(localRoots, nil, writableRoots) + kc.SetServiceRoots(localRoots, writableLocalRoots, nil) _, replicas, err := kc.PutB([]byte("foo")) c.Check(err, Equals, InsufficientReplicasError) c.Check(replicas, Equals, 0) - - log.Printf("TestPutBWithNoWritableRoots done") } diff --git a/sdk/go/keepclient/support.go b/sdk/go/keepclient/support.go index b989c5d4bf..7b96341823 100644 --- a/sdk/go/keepclient/support.go +++ b/sdk/go/keepclient/support.go @@ -94,7 +94,7 @@ func (this *KeepClient) DiscoverKeepServers() error { listed := make(map[string]bool) localRoots := make(map[string]string) gatewayRoots := make(map[string]string) - writableRoots := make(map[string]string) + writableLocalRoots := make(map[string]string) for _, service := range m.Items { scheme := "http" @@ -118,7 +118,7 @@ func (this *KeepClient) DiscoverKeepServers() error { } if service.ReadOnly == false { - writableRoots[service.Uuid] = url + writableLocalRoots[service.Uuid] = url } // Gateway services are only used when specified by @@ -135,7 +135,7 @@ func (this *KeepClient) DiscoverKeepServers() error { this.setClientSettingsStore() } - this.SetServiceRoots(localRoots, gatewayRoots, writableRoots) + this.SetServiceRoots(localRoots, writableLocalRoots, gatewayRoots) return nil } @@ -219,7 +219,7 @@ func (this KeepClient) putReplicas( requestId := fmt.Sprintf("%x", md5.Sum([]byte(locator+time.Now().String())))[0:8] // Calculate the ordering for uploading to servers - sv := NewRootSorter(this.WritableRoots(), hash).GetSortedRoots() + sv := NewRootSorter(this.WritableLocalRoots(), hash).GetSortedRoots() // The next server to try contacting next_server := 0 diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py index 2846630437..1e0917244e 100644 --- a/sdk/python/arvados/keep.py +++ b/sdk/python/arvados/keep.py @@ -648,10 +648,7 @@ class KeepClient(object): 'uuid': 'proxy', '_service_root': proxy, }] - self._writable_services = [{ - 'uuid': 'proxy', - '_service_root': proxy, - }] + self._writable_services = self._keep_services self.using_proxy = True self._static_services_list = True else: @@ -759,7 +756,7 @@ class KeepClient(object): # for this locator, and return their service_roots (base URIs) # in that order. use_services = self._keep_services - if (need_writable == True): + if need_writable: use_services = self._writable_services sorted_roots.extend([ svc['_service_root'] for svc in sorted( diff --git a/services/api/db/migrate/20150512193020_read_only_on_keep_services.rb b/services/api/db/migrate/20150512193020_read_only_on_keep_services.rb index 1778b075d3..f86e4711f2 100644 --- a/services/api/db/migrate/20150512193020_read_only_on_keep_services.rb +++ b/services/api/db/migrate/20150512193020_read_only_on_keep_services.rb @@ -1,9 +1,5 @@ class ReadOnlyOnKeepServices < ActiveRecord::Migration - def up + def change add_column :keep_services, :read_only, :boolean, null: false, default: false end - - def down - remove_column :keep_services, :read_only - end end diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go index b70b933368..5bd832b511 100644 --- a/services/keepproxy/keepproxy_test.go +++ b/services/keepproxy/keepproxy_test.go @@ -118,10 +118,10 @@ func runProxy(c *C, args []string, port int, bogusClientToken bool) keepclient.K locals := map[string]string{ "proxy": fmt.Sprintf("http://localhost:%v", port), } - writables := map[string]string{ + writableLocals := map[string]string{ "proxy": fmt.Sprintf("http://localhost:%v", port), } - kc.SetServiceRoots(locals, nil, writables) + kc.SetServiceRoots(locals, writableLocals, nil) c.Check(kc.Using_proxy, Equals, true) c.Check(len(kc.LocalRoots()), Equals, 1) for _, root := range kc.LocalRoots() { diff --git a/services/keepstore/pull_worker.go b/services/keepstore/pull_worker.go index 2707b98a0a..3d67cf2c1e 100644 --- a/services/keepstore/pull_worker.go +++ b/services/keepstore/pull_worker.go @@ -46,7 +46,7 @@ func PullItemAndProcess(pullRequest PullRequest, token string, keepClient *keepc for _, addr := range pullRequest.Servers { service_roots[addr] = addr } - keepClient.SetServiceRoots(service_roots, nil, keepClient.WritableRoots()) + keepClient.SetServiceRoots(service_roots, nil, nil) // Generate signature with a random token expires_at := time.Now().Add(60 * time.Second) -- 2.30.2