From ff7cae6d3433a6bcfd0729fe7c3e2f0010d72e19 Mon Sep 17 00:00:00 2001 From: cmasone-attic Date: Wed, 19 Apr 2017 21:34:20 -0700 Subject: [PATCH] Merge chunks.RootTracker interface into chunks.ChunkStore (#3408) You can't fully specify RootTracker without referring to the ChunkStore interface, so they should just merge. Fixes #3402 --- cmd/noms/noms_root.go | 7 ++-- go/chunks/chunk_store.go | 68 +++++++++++++------------------- go/config/resolver.go | 13 ------ go/datas/completeness_checker.go | 2 +- go/datas/database_common.go | 8 +++- go/nbs/root_tracker_test.go | 2 +- 6 files changed, 40 insertions(+), 60 deletions(-) diff --git a/cmd/noms/noms_root.go b/cmd/noms/noms_root.go index 0e85180331..1eed7bec2d 100644 --- a/cmd/noms/noms_root.go +++ b/cmd/noms/noms_root.go @@ -43,10 +43,10 @@ func runRoot(args []string) int { } cfg := config.NewResolver() - rt, err := cfg.GetRootTracker(args[0]) + cs, err := cfg.GetChunkStore(args[0]) d.CheckErrorNoUsage(err) - currRoot := rt.Root() + currRoot := cs.Root() if updateRoot == "" { fmt.Println(currRoot) @@ -62,6 +62,7 @@ func runRoot(args []string) int { return 1 } + // If BUG 3407 is correct, we might be able to just take cs and make a Database directly from that. db, err := cfg.GetDatabase(args[0]) d.CheckErrorNoUsage(err) defer db.Close() @@ -85,7 +86,7 @@ Continue? return 0 } - ok = rt.Commit(h, currRoot) + ok = cs.Commit(h, currRoot) if !ok { fmt.Fprintln(os.Stderr, "Optimistic concurrency failure") return 1 diff --git a/go/chunks/chunk_store.go b/go/chunks/chunk_store.go index 5f92f5592d..d147b729c2 100644 --- a/go/chunks/chunk_store.go +++ b/go/chunks/chunk_store.go @@ -10,48 +10,9 @@ import ( "github.com/attic-labs/noms/go/hash" ) -// ChunkStore is the core storage abstraction in noms. We can put data anyplace we have a -// ChunkStore implementation for. +// ChunkStore is the core storage abstraction in noms. We can put data +// anyplace we have a ChunkStore implementation for. type ChunkStore interface { - ChunkSource - RootTracker -} - -// Factory allows the creation of namespaced ChunkStore instances. The details -// of how namespaces are separated is left up to the particular implementation -// of Factory and ChunkStore. -type Factory interface { - CreateStore(ns string) ChunkStore - - // Shutter shuts down the factory. Subsequent calls to CreateStore() will fail. - Shutter() -} - -// RootTracker allows querying and management of the root of an entire tree of -// references. The "root" is the single mutable variable in a ChunkStore. It -// can store any hash, but it is typically used by higher layers (such as -// Database) to store a hash to a value that represents the current state and -// entire history of a database. -// TODO: Does having a separate RootTracker make sense anymore? BUG 3402 -type RootTracker interface { - // Rebase brings this RootTracker into sync with the persistent storage's - // current root. - Rebase() - - // Root returns the root of the database as of the time the RootTracker - // was opened or the most recent call to Rebase. - Root() hash.Hash - - // Commit atomically attempts to persist all novel Chunks and update the - // persisted root hash from last to current. If last doesn't match the - // root in persistent storage, returns false. - // TODO: is last now redundant? Maybe this should just try to update from - // the cached root to current? - Commit(current, last hash.Hash) bool -} - -// ChunkSource is a place chunks live. -type ChunkSource interface { // Get the Chunk for the value of the hash in the store. If the hash is // absent from the store nil is returned. Get(h hash.Hash) Chunk @@ -89,5 +50,30 @@ type ChunkSource interface { // call Flush() concurrently with Put() or PutMany(). Flush() + // Rebase brings this ChunkStore into sync with the persistent storage's + // current root. + Rebase() + + // Root returns the root of the database as of the time the ChunkStore + // was opened or the most recent call to Rebase. + Root() hash.Hash + + // Commit atomically attempts to persist all novel Chunks and update the + // persisted root hash from last to current. If last doesn't match the + // root in persistent storage, returns false. + // TODO: is last now redundant? Maybe this should just try to update from + // the cached root to current? + Commit(current, last hash.Hash) bool + io.Closer } + +// Factory allows the creation of namespaced ChunkStore instances. The details +// of how namespaces are separated is left up to the particular implementation +// of Factory and ChunkStore. +type Factory interface { + CreateStore(ns string) ChunkStore + + // Shutter shuts down the factory. Subsequent calls to CreateStore() will fail. + Shutter() +} diff --git a/go/config/resolver.go b/go/config/resolver.go index b09f8c10db..b37e0226ee 100644 --- a/go/config/resolver.go +++ b/go/config/resolver.go @@ -104,19 +104,6 @@ func (r *Resolver) GetChunkStore(str string) (chunks.ChunkStore, error) { return sp.NewChunkStore(), nil } -// Resolve string to a RootTracker. Like ResolveDatabase, but returns a RootTracker instead -func (r *Resolver) GetRootTracker(str string) (chunks.RootTracker, error) { - sp, err := spec.ForDatabase(r.verbose(str, r.ResolveDbSpec(str))) - if err != nil { - return nil, err - } - var rt chunks.RootTracker = sp.NewChunkStore() - if rt == nil { - rt = datas.NewHTTPChunkStore(sp.String(), "") - } - return rt, nil -} - // Resolve string to a dataset. If a config is present, // - if no db prefix is present, assume the default db // - if the db prefix is an alias, replace it diff --git a/go/datas/completeness_checker.go b/go/datas/completeness_checker.go index 45dcccc75d..6e2e5d5b25 100644 --- a/go/datas/completeness_checker.go +++ b/go/datas/completeness_checker.go @@ -30,7 +30,7 @@ func (cc *completenessChecker) AddRefs(v types.Value) { // PanicIfDangling panics if any refs in unresolved point to chunks not // present in cs. -func (cc *completenessChecker) PanicIfDangling(cs chunks.ChunkSource) { +func (cc *completenessChecker) PanicIfDangling(cs chunks.ChunkStore) { present := cs.HasMany(cc.unresolved) absent := hash.HashSlice{} for h := range cc.unresolved { diff --git a/go/datas/database_common.go b/go/datas/database_common.go index 3d2fec0b3b..d216cb4f26 100644 --- a/go/datas/database_common.go +++ b/go/datas/database_common.go @@ -17,7 +17,7 @@ import ( type databaseCommon struct { *types.ValueStore cch *cachingChunkHaver - rt chunks.RootTracker + rt rootTracker rootHash hash.Hash datasets *types.Map } @@ -27,6 +27,12 @@ var ( ErrMergeNeeded = errors.New("Dataset head is not ancestor of commit") ) +// rootTracker is a narrowing of the ChunkStore interface, to keep Database disciplined about working directly with Chunks +type rootTracker interface { + Root() hash.Hash + Commit(current, last hash.Hash) bool +} + func newDatabaseCommon(cs chunks.ChunkStore) databaseCommon { return databaseCommon{ ValueStore: types.NewValueStore(cs), diff --git a/go/nbs/root_tracker_test.go b/go/nbs/root_tracker_test.go index 220555a023..7974ac098b 100644 --- a/go/nbs/root_tracker_test.go +++ b/go/nbs/root_tracker_test.go @@ -155,7 +155,7 @@ func createMemTable(chunks [][]byte) *memTable { return mt } -func assertDataInStore(slices [][]byte, store chunks.ChunkSource, assert *assert.Assertions) { +func assertDataInStore(slices [][]byte, store chunks.ChunkStore, assert *assert.Assertions) { for _, data := range slices { assert.True(store.Has(chunks.NewChunk(data).Hash())) }