Revert "Fix ZK racyness when reading latest config (#105)"

This reverts commit 72704da.

config/zookeeper_persister.go

@@ -221,11 +221,7 @@ func (z *ZkConfigPersister) currentConfigEventListener() (<-chan zk.Event, error
 	}
 
 	configs := make(map[string]*pb.ServiceConfig)
-	latestHashVersion := int32(0)
-	var latestHash string
 
-	// Iterate over all children in this path for 2 reasons: add all of them to the historical version map, and to work
-	// out which is the most recent, to use as the current version.
 	for _, hash := range children {
 		path := fmt.Sprintf("%s/%s", z.path, hash)
 		data, _, err := z.conn.Get(path)
@@ -237,27 +233,14 @@ func (z *ZkConfigPersister) currentConfigEventListener() (<-chan zk.Event, error
 
 		configs[hash] = &pb.ServiceConfig{}
 		proto.Unmarshal(data, configs[hash])
-
-		// TODO(manik) replace this with a ZK node that tracks the latest version rather than deserializing each node
-		// TODO(manik) we can currently have multiple hashes with the same version; this needs to be fixed at the time
-		// of writing
-		if configs[hash].Version >= latestHashVersion {
-			latestHashVersion = configs[hash].Version
-			latestHash = hash
-		}
 	}
 
 	if z.initialized {
 		logging.Printf("Re-establishing zookeeper watch on %v", z.path)
 	} else {
 		logging.Printf("Establishing zookeeper watch on %v", z.path)
 	}
-
-	// Ignoring the response to getting the contents of the watch. We don't care which node triggered the watch, since
-	// we're working out the "most recent" config above by iterating over all available configurations and sorting by
-	// the configuration's Version field. All we care about here is the channel watching the ZK path, to be notified of
-	// future changes.
-	_, _, ch, err := z.conn.GetW(z.path)
+	config, _, ch, err := z.conn.GetW(z.path)
 
 	if err != nil {
 		logging.Printf("Received error from zookeeper when fetching %s: %+v", z.path, err)
@@ -268,9 +251,7 @@ func (z *ZkConfigPersister) currentConfigEventListener() (<-chan zk.Event, error
 	defer z.Unlock()
 
 	z.configs = configs
-	z.config = latestHash
-
-	logging.Printf("Setting latest config hash to %v (version %v)", z.config, latestHashVersion)
+	z.config = string(config)
 
 	select {
 	case z.watcher <- struct{}{}:

config/zookeeper_persister_test.go

@@ -6,6 +6,7 @@ package config
 import (
 	"fmt"
 	"io/ioutil"
+	"reflect"
 	"testing"
 	"time"
 
@@ -120,7 +121,6 @@ func TestSetAndNotify(t *testing.T) {
 
 	cfg := NewDefaultServiceConfig()
 	cfg.Namespaces["foo"] = NewDefaultNamespaceConfig("foo")
-	cfg.Version++
 
 	helpers.CheckError(t, p.PersistAndNotify("", cfg))
 
@@ -213,7 +213,11 @@ func TestReadingStaleVersions(t *testing.T) {
 	helpers.CheckError(t, err)
 
 	if latest.Version != 202 {
-		t.Fatalf("Expected latest version = 200, got %v", latest.Version)
+		t.Logf("Latest config should have version 202, but was %v. Persister latest hash == %v; historical == %+v",
+			latest.Version, p.(*ZkConfigPersister).config, reflect.ValueOf(p.(*ZkConfigPersister).configs).MapKeys())
+
+		// TODO(manik) this is a known problem that needs addressing.
+		// t.FailNow()
 	}
 }