CacheDB #67

jaekwon · 2017-10-24T21:39:34Z

This PR is to make the tendermint/tmlibs/db CacheWrap() compatible.
We could in the future use cosmos/cosmos-sdk/state/kvcache.go (or whatever it becomes), but it's not clear whether we want to make the Cosmos KVStore/IterKVStore interfaces (designed for Merkle stores) compatible with the Tendermint DB interface (designed for LevelDB etc). Until we can unify them lets keep them separate like this for now.

Also fixes a bug where MemDB keys weren't being ordered before Merkle-rooting.
Also fixes c-leveldb compatibility. You need to run brew install leveldb or equivalent to run the tests.

NOTE: We need c-leveldb support so we should keep them as part of the tests, and fix our CI environment as necessary, instead of crippling our tests.

ebuchman

Various exported types could use some comment docs.

We can probably avoid append in many cases for efficiency.

Otherwise cool

ebuchman · 2017-10-24T21:53:05Z

db/cache_db.go

+	// Not the best, but probably not a bottleneck depending.
+	keys := []string{}
+	for key, _ := range db.cache {
+		keys = append(keys, key)


at the least we can do this without append by allocating up front

ebuchman · 2017-10-24T22:02:44Z

db/cache_db.go

+	db.mtx.Lock()
+	defer db.mtx.Unlock()
+
+	db.cache[string(key)] = cDBValue{nil, true}


why not DeleteNoLock?

ebuchman · 2017-10-24T22:02:48Z

db/cache_db.go

+	db.mtx.Lock()
+	defer db.mtx.Unlock()
+
+	db.cache[string(key)] = cDBValue{nil, true}


ebuchman · 2017-10-24T22:03:56Z

db/cache_db.go

+
+	fmt.Println("CacheDB\ncache:\n")
+	for key, value := range db.cache {
+		fmt.Printf("[%X]:\t[%X]\n", []byte(key), value)


do we need the []byte(key)? I think key suffices

ebuchman · 2017-10-25T03:31:22Z

db/mem_batch.go

+	defer mtx.Unlock()
+
+	for _, op := range mBatch.ops {
+		if op.opType == opTypeSet {


maybe nicer to use a switch

ebuchman · 2017-10-25T03:32:14Z

db/mem_db.go

-		if strings.HasPrefix(key, string(prefix)) {
-			it.keys = append(it.keys, key)
-		}
+		it.keys = append(it.keys, key)


can allocate up front

ebuchman · 2017-10-25T04:56:40Z

merkle/kvpairs.go

+
+func MakeSortedKVPairs(m map[string]interface{}) []Hashable {
+	kvPairs := []KVPair{}
+	for k, v := range m {


allocate up front

ebuchman · 2017-10-25T04:57:30Z

merkle/simple_map.go

+func (sm *SimpleMap) Hash() []byte {
+	sm.kvz.Sort()
+	kvPairsH := []Hashable{}
+	for _, kvp := range sm.kvz {


allocate up front

ebuchman · 2017-10-25T04:58:14Z

merkle/simple_map_test.go

+	}
+	{
+		db := NewSimpleMap()
+		db.Set("key1", "value1")


duplicate test case ?

melekes · 2017-10-25T08:18:00Z

make metalinter_test failed with:

db/cache_db.go:134:33:warning: unnecessary conversion (unconvert)
db/backend_test.go:8:9:warning: unused variable or constant could not import github.com/tendermint/go-common (cannot find package "github.com/tendermint/go-common" in any of: (varcheck)
db/backend_test.go:17:18:warning: unused variable or constant undeclared name: common (varcheck)
db/cache_db.go:89::error: Println call ends with newline (vet)
db/cache_db.go:91::error: arg value for printf verb %X of wrong type: db.cDBValue (vet)
db/cache_db.go:93::error: Println call ends with newline (vet)
merkle/simple_proof.go:60::error: unreachable code (vet)
db/backend_test.go:8:9:warning: could not import github.com/tendermint/go-common (cannot find package "github.com/tendermint/go-common" in any of: (megacheck)
db/backend_test.go:17:18:warning: undeclared name: common (megacheck)

melekes · 2017-10-25T08:20:38Z

db/backend_test.go

+	common "github.com/tendermint/go-common"
+)
+
+func print(x []byte) {


don't see where it's being used...

Didn't this repo die long ago? To be reborn as tmlibs

melekes · 2017-10-25T08:23:21Z

db/c_level_db.go

+}
+
+func (c cLevelDBIterator) Next() bool {
+	c.Next()


infinite recursion? maybe c.(*levigo.Iterator).Next()

nice catch!

c.Iterator.Next() should work...

Or perhaps let's make it explicit to

type cLevelDBIterator struct { iter *levigo.Iterator } func (c cLevelDBIterator) Next() bool { c.iter.Next() }

to avoid such problems that we previously had with 0b22b27 where the problem slipped through the cracks?

melekes · 2017-10-25T08:30:34Z

db/db.go

@@ -3,15 +3,14 @@ package db
 import . "github.com/tendermint/tmlibs/common"

 type DB interface {
-	Get([]byte) []byte
+	Get([]byte) []byte // NOTE: returns nil iff never set or deleted.


iff means if and only if

did not know, thanks!

melekes · 2017-10-25T08:31:19Z

db/go_level_db.go

@@ -155,10 +154,6 @@ func (db *GoLevelDB) Iterator() Iterator {
 	return &goLevelDBIterator{db.db.NewIterator(nil, nil)}
 }

-func (db *GoLevelDB) IteratorPrefix(prefix []byte) Iterator {


why delete? nobody using it?

I'm using this in iavl.

I think we should take a look at levigo/goleveldb's iterators and what's supported there... first.

LeviGo supports Seek... https://github.com/jmhodges/levigo/blob/master/iterator.go#L121

So does Goleveldb: https://godoc.org/github.com/syndtr/goleveldb/leveldb/iterator#IteratorSeeker

Lets adopt these as tmlibs/db.Iterator, and make MemDB support it too...

I'll write an exposed module function called "IteratePrefix" that takes an iterator and calls Seek() accordingly to get the same effect.

melekes · 2017-10-25T08:37:55Z

merkle/simple_map.go

+func (sm *SimpleMap) Hash() []byte {
+	sm.kvz.Sort()
+	kvPairsH := []Hashable{}
+	for _, kvp := range sm.kvz {


and what if the value is not Hashable? I am asking because in KVPair#Hash there is an if if kvH, ok := kv.Value.(Hashable); ok {

The KVPair is a Hashable that knows how to deal with binary objects.

adrianbrink · 2017-10-25T09:42:14Z

make metalinter_test fails. Either we have to fix up all the warnings once or we disable it for now but run it locally and fix a little bit everytime.

cloudhead

We make heavy use of IteratorPrefix in iavl and it's all-around useful for iterating over ordered key-spaces, so we should keep it.

cloudhead · 2017-10-25T09:51:00Z

db/db.go

 	Set([]byte, []byte)
 	SetSync([]byte, []byte)
 	Delete([]byte)
 	DeleteSync([]byte)
 	Close()
 	NewBatch() Batch
 	Iterator() Iterator
-	IteratorPrefix([]byte) Iterator


We need this for iavl.

cloudhead · 2017-10-25T09:51:38Z

db/go_level_db.go

@@ -155,10 +154,6 @@ func (db *GoLevelDB) Iterator() Iterator {
 	return &goLevelDBIterator{db.db.NewIterator(nil, nil)}
 }

-func (db *GoLevelDB) IteratorPrefix(prefix []byte) Iterator {


I'm using this in iavl.

cloudhead · 2017-10-25T09:56:38Z

merkle/simple_map_test.go

+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	// "github.com/stretchr/testify/require"


We can delete this.

ebuchman · 2017-10-25T13:05:39Z

Makefile

@@ -33,15 +33,15 @@ metalinter_test: ensure_tools
 		--enable=goconst \
 		--enable=gosimple \
 	 	--enable=ineffassign \
-	   	--enable=interfacer \
+	  --enable=interfacer \


why did all this spacing get weird?

Mh, some had two spaces instead of tabs. Everything is now intended with 2 tabs.

ebuchman · 2017-10-25T13:06:42Z

db/c_level_db.go

@@ -49,6 +51,8 @@ func NewCLevelDB(name string, dir string) (*CLevelDB, error) {
 	return database, nil
 }

+// Get gets the value for the given key. It returns ErrNotFound if the DB does not contains the


This doesn't sound right. The function panics on err, and should return nil if the key is not there. Is that not what it does?

You are right. That doesn't sound right. I think I might have thought about something else while typing this :-)

These comments are not necessary, we should just comment the interface functions (for DB and Iterator)

ebuchman · 2017-10-25T13:07:07Z

db/c_level_db.go

@@ -19,13 +19,15 @@ func init() {
 	registerDBCreator(CLevelDBBackendStr, dbCreator, false)
 }

+// CLevelDB provides access to the C implementation of LevelDB


We should add a XXX comment that these methods panic on error from the underlying DB

ebuchman · 2017-10-25T13:09:25Z

db/cache_db.go

@@ -37,63 +41,77 @@ func (db *CacheDB) Get(key []byte) []byte {
 	return db.parent.Get(key)
 }

+// Set sets the value for the given key in the cache. It does not write the value to the
+// underlying database. Set does not incur a performance penalty compared to SetNoLock.


What's all this about performance penalty? These functions lock a mutex while XxxNoLock does not

ebuchman · 2017-10-25T13:10:18Z

db/cache_db.go

 func (db *CacheDB) DeleteNoLock(key []byte) {
 	db.cache[string(key)] = cDBValue{nil, true}
 }

+// Close closes the underlying database by deallocating the handle. Any attempts to use the DB
+// after close will result in a panic.


This probably isn't true, maybe we should clarify that the db should not be used after Close. But it won't panic in this case I don't thiknk ...

That's right. It depends on the backing database.

odeke-em · 2017-10-26T03:18:04Z

.gitignore

@@ -1,4 +1,5 @@
 *.swp


We could also do

*.sw[op]

and it's awesome to know that you too @jaekwon use Vim!

odeke-em · 2017-10-26T03:18:57Z

Makefile

@@ -11,7 +11,7 @@ all: test
 NOVENDOR = go list github.com/tendermint/tmlibs/... | grep -v /vendor/

 test:
-	go test `glide novendor`
+	go test -tags gcc `glide novendor`


Am not too sure if we should always be including the gcc tag, since on my Mac this is going to trip out.

How come it trips on your Mac? It runs fine on macOS 10.13 for me.

Works for me... go 1.8.1, 10.12.x

Aha nice, my Mac has issues with gcc tags but I'll try again.

This was where I first tripped out tendermint/merkleeyes#39 (comment) 2 months ago.

odeke-em · 2017-10-26T03:31:50Z

db/cache_db.go

+	if ok {
+		return value.value
+	}
+	return db.parent.Get(key)


Hmm, I don't see where we memoize the cache-missed value for this, perhaps let's do this.

value, ok := db.cache[string(key)] if !ok { data := db.parent.Get(key) value = cDBValue{value: data} db.cache[string(key)] = value } return value.value

Also should we perhaps check if deleted { return nil }?

I was wondering about that too. Maybe @jaekwon can clarify how this functionality is supposed to work. I got the understanding that you had to explicitly add values to the cache.

if deleted, value.value will be nil.

odeke-em · 2017-10-26T03:33:57Z

db/cache_db.go

+}
+
+// SetNoLock sets the value for the given key in the database. It does not write the value to the
+// underlying database. SetNoLock implements atomicSetDeleter.


How come we are referring to an atomicSetDeleter unexported interface?
Perhaps could we just do a compile type assertion and not mention it? e.g

var _ atomicSetDeleter = (*CacheDB)(nil)

odeke-em · 2017-10-26T03:35:19Z

db/cache_db.go

+// DeleteNoLock deletes the value for the given key. It does not delete the value from the
+// underlying database. DeleteNoLock implements atomicSetDeleter.
+func (db *CacheDB) DeleteNoLock(key []byte) {
+	db.cache[string(key)] = cDBValue{nil, true}


I think we need mutex protection here :)

But the function is called DeleteNoLock

Ahah, thanks for that catch @cloudhead I made that suggestion without noticing that.

odeke-em · 2017-10-26T03:36:01Z

db/cache_db.go

+
+	stats := make(map[string]string)
+	stats["cache.size"] = fmt.Sprintf("%d", len(db.cache))
+	mergeStats(db.parent.Stats(), stats, "parent.")


odeke-em · 2017-10-26T03:38:26Z

db/cache_db.go

+
+	// Clear the cache
+	db.cache = make(map[string]cDBValue)
+}


Very nice routine!

odeke-em · 2017-10-26T03:43:39Z

db/mem_db.go

+	delete(db.db, string(key))
+}
+
+// Close is a noop.


Thanks for the doc!

odeke-em · 2017-10-26T03:48:09Z

merkle/kvpairs.go

+func (kvps KVPairs) Len() int           { return len(kvps) }
+func (kvps KVPairs) Less(i, j int) bool { return kvps[i].Key < kvps[j].Key }
+func (kvps KVPairs) Swap(i, j int)      { kvps[i], kvps[j] = kvps[j], kvps[i] }
+func (kvps KVPairs) Sort()              { sort.Sort(kvps) }


I just tagged you on the internal thread about the support for Go1.8+ where we'll later on just use sort.Slice that is

sort.Slice(kvPairs, func(i, j int) bool { kvi, kvj := kvPairs[i], kvPairs[j] return kvi.Key < kvj.Key })

iavl only compiles with go 1.8+
I questioned this, and bucky, alexis, and adrian all spoke in favor of making this a requirement. It is commented in cosmos-sdk (due to iavl dependency), let's just update everything to 1.8

odeke-em · 2017-10-26T03:50:14Z

merkle/simple_proof.go

+	if computedHash == nil {
+		return false
+	}
+	if !bytes.Equal(computedHash, rootHash) {


return computedHash != nil && bytes.Equal(computedHash, rootHash)

odeke-em · 2017-10-26T03:55:09Z

merkle/simple_proof.go

+	}
+	switch total {
+	case 0:
+		panic("Cannot call computeHashFromAunts() with 0 total")


And for a nerdy detour and if you get bored: this is one of my favorite compiler checks. We don't need the return nil below because panic is a terminating statement :) as per

https://golang.org/ref/spec#Return_statements

https://golang.org/ref/spec#Terminating_statements
See https://play.golang.org/p/Uywqmi2m4G

odeke-em

Thank you @jaekwon for the PR, I've added a few suggestions, PTAL.

melekes · 2017-10-27T07:51:53Z

db/c_level_db.go

+}
+
+func (c cLevelDBIterator) Close() {
+	c.Close()


c.itr.Close()?

jesus. thanks.

melekes · 2017-10-27T07:52:17Z

db/c_level_db.go

+}
+
+func (c cLevelDBIterator) GetError() error {
+	return c.GetError()


c.itr.GetError()?

melekes · 2017-10-27T07:53:47Z

db/c_level_db_test.go

@@ -7,7 +7,7 @@ import (
 	"fmt"
 	"testing"

-	. "github.com/tendermint/tmlibs/common"
+	"github.com/tendermint/tmlibs/common"


the convention is cmn

melekes · 2017-10-27T07:55:50Z

db/cache_db.go

+
+	value, ok := db.cache[string(key)]
+	if !ok {
+		fmt.Println("1", string(key))


Are these debug statements?

melekes · 2017-10-27T07:57:35Z

db/common_test.go

+	"github.com/tendermint/tmlibs/common"
+)
+
+func checkValid(t *testing.T, itr Iterator, expected bool) {


❤️ these wrappers

melekes · 2017-10-27T07:59:57Z

db/common_test.go

+}
+
+func checkNextPanics(t *testing.T, itr Iterator) {
+	defer func() {


https://godoc.org/github.com/stretchr/testify/assert#Panics

assert.Panics(t, func(){ itr.Next() }, "Next expected panic but didn't")

melekes · 2017-10-27T08:00:08Z

db/common_test.go

+}
+
+func checkPrevPanics(t *testing.T, itr Iterator) {
+	defer func() {


melekes · 2017-10-27T08:01:12Z

db/common_test.go

+}
+
+func newTempDB(t *testing.T, backend string) (db DB) {
+	dir, dirname := common.Tempdir(fmt.Sprintf("test_go_iterator"))


no need for fmt.Sprintf

melekes · 2017-10-27T08:28:47Z

db/go_level_db.go

+	}
+	if !it.source.Valid() {
+		it.invalid = true
+		return false


if it.invalid { return false } it.invalid = !it.source.Valid() return !it.invalid

melekes · 2017-10-27T08:38:11Z

db/util.go

+	return pi.itr.Value()
+}
+
+func (pi *prefixIterator) Close()          {}


shouldn't we delegate to pi.itr? pi.itr.Close()

odeke-em · 2017-10-28T04:34:14Z

db/c_level_db_test.go

@@ -7,7 +7,7 @@ import (
 	"fmt"
 	"testing"

-	. "github.com/tendermint/tmlibs/common"
+	cmn "github.com/tendermint/tmlibs/common"


odeke-em · 2017-10-28T04:36:01Z

db/cache_db.go

+	defer db.mtx.Unlock()
+
+	value, ok := db.cache[string(key)]
+	if !ok {


One other check we might need, I had mentioned in the previous review but in later comments: we perhaps want to check that value.deleted is not set lest that's a discarded value in the cache i.e

if !ok || value.deleted { }

What do y'all think?

not value.deleted... if value.deleted in the cache, we don't want to query the base store.

if value.deleted, value.value is nil btw.

odeke-em · 2017-10-28T04:38:21Z

db/cache_db.go

+
+func (db *CacheDB) Set(key []byte, value []byte) {
+	db.mtx.Lock()
+	defer db.mtx.Unlock()


Now here I am being a bit painful about this, but I believe we can squeeze even better perhaps by avoiding defers whenever we can, as they are ~2X slower than non-defers, please see https://gist.github.com/odeke-em/f8fb3a8695b33dd06a700fc3473f3e62.
Could we please make all these:

db.mtx.Lock() db.SetNoLock(key, value) db.mtx.Unlock()

I insist that we keep the defers until we find that it's a bottleneck... it's defensive programming that IMO is well worth it.

odeke-em · 2017-10-28T04:38:36Z

db/cache_db.go

+
+func (db *CacheDB) SetSync(key []byte, value []byte) {
+	db.mtx.Lock()
+	defer db.mtx.Unlock()


Ditto about the defers

odeke-em · 2017-10-28T04:38:45Z

db/cache_db.go

+
+func (db *CacheDB) Delete(key []byte) {
+	db.mtx.Lock()
+	defer db.mtx.Unlock()


Ditto about the defers

odeke-em · 2017-10-28T04:38:52Z

db/cache_db.go

+
+func (db *CacheDB) DeleteSync(key []byte) {
+	db.mtx.Lock()
+	defer db.mtx.Unlock()


Ditto about the defers

odeke-em · 2017-10-28T04:38:59Z

db/cache_db.go

+
+func (db *CacheDB) Close() {
+	db.mtx.Lock()
+	defer db.mtx.Unlock()


Ditto about the defers

odeke-em · 2017-10-28T04:49:47Z

db/cache_db.go

+	db.parent.Close()
+}
+
+func (db *CacheDB) Print() {


So while Print is useful from an external perspective, it is not that easy to test. IMHO we could do something like

func (db *CacheDB) Fprint(w io.Writer) { db.mtx.Lock() defer db.mtx.Unlock() fmt.Fprintln(w, "CacheDB\ncache:") for key, value := range db.cache { fmt.Fprintf(w, "[%X]:\t[%v\n", key, value) } fmt.Fprintln(w, "\nparent:") db.parent.Print() } func (db *CacheDB) Print() { db.Fprint(os.Stdout) }

and then properly test the order and expected out.

I wouldn't want to test Print functions TBH.

odeke-em · 2017-10-28T04:53:23Z

db/db.go

@@ -24,17 +25,59 @@ type Batch interface {
 	Write()
 }

+/*
+	Usage:


odeke-em · 2017-10-28T04:54:40Z

db/go_level_db_test.go

@@ -6,7 +6,7 @@ import (
 	"fmt"
 	"testing"

-	. "github.com/tendermint/tmlibs/common"
+	"github.com/tendermint/tmlibs/common"


Minor nit, the convention seemingly used is cmn instead of common ie

cmn "github.com/tendermint/tmlibs/common"

odeke-em · 2017-10-28T04:55:46Z

db/mem_batch.go

+type opType int
+
+const (
+	opTypeSet    = 1


We missed setting the opType here so

const ( opTypeSet opType = 1 opTypeDelete opType = 2 )

odeke-em · 2017-10-28T04:56:06Z

db/mem_batch.go

+}
+
+func (mBatch *memBatch) Write() {
+	mtx := mBatch.db.Mutex()


odeke-em · 2017-10-28T04:56:46Z

db/mem_batch.go

+	mBatch.ops = append(mBatch.ops, operation{opTypeDelete, key, nil})
+}
+
+func (mBatch *memBatch) Write() {


Write? Flush perhaps? Am just bikeshedding on the name, it doesn't matter much :)

You mean you're yak-shaving the lingo?

odeke-em · 2017-10-28T04:57:06Z

db/mem_db.go

@@ -28,15 +29,30 @@ func (db *MemDB) Get(key []byte) []byte {
 	return db.db[string(key)]
 }

+func (db *MemDB) GetOK(key []byte) ([]byte, bool) {
+	db.mtx.Lock()
+	defer db.mtx.Unlock()


Ditto here about an unnecessary defer.

odeke-em · 2017-10-28T04:57:29Z

db/mem_db.go

 }

 func (db *MemDB) SetSync(key []byte, value []byte) {
 	db.mtx.Lock()
 	defer db.mtx.Unlock()
+	db.SetNoLock(key, value)


Ditto here about the unnecessary defer.

odeke-em

Awesome, thanks @jaekwon for the round of review addressing. I just have one last set of requests then am gucci to approve, most especially about some defers that we could shed, PTAL.

I undid adrian's pushes.

ethanfrey

In general, it looks quite nice, and I like the design.

Two points about CacheWrap, returning something more useful than interface{} and separating read and write caches. Otherwise, I like it.

ethanfrey · 2017-11-01T13:16:35Z

db/db.go


+	// Seek moves the iterator the position of the key given or, if the key


So seek is like the replacement for IteratorPrefix?
Can you add in the example a check to show how to iterate in a range? Cuz if we use seek to find the beginning, there is probably an end we also want to see.

But yeah, Seek() with adding Prev() as well as Next() seems like a nice generalization that allows a lot more flexibility to whoever uses this. Just show those example usages.

No, the replacement for IteratorPrefix is db.IteratePrefix(db DB, prefix []byte) Iterator.

Besides using db.IteratePrefix, You'll just have to manually stop iteration when you reach the end that you desire

ethanfrey · 2017-11-01T13:24:18Z

db/db.go

 	Stats() map[string]string
+
+	// CacheWrap wraps the DB w/ a CacheDB.


Why does this return interface{}? It provides no guarantees.

In cachedb_test.go, I see this line often:
cdb := mem.CacheWrap().(*CacheDB)

If the only way to use it is to cast it to a particular type, then just force that type.
If you want people to be able to use the CacheWrap without forcing a given type, then return a usable interface,that exposes some subset of the DB interface.

Actually, it seems *CacheDB implements the entire DB interface, why not return it there? Or remove a method or two that you don't want to require from Caches...

type Cache interface { ..... } type DB interface { Cache NewBatch() Batch Iterate() Iterator }

then we could just use cdb := mem.CacheWrap()

Maybe. But CacheWrap also exists in the SDK for the merkle stores, and those don't conform to tmlibs/db.DB...

ethanfrey · 2017-11-01T13:30:06Z

db/cache_db_test.go

+	require.Panics(t, func() { cdb2.Write() })
+}
+
+func TestCacheDBNested(t *testing.T) {


Good tests. Maybe add a few more.

What if we make two caches A, and B from one parent, and then try to write them both?
Is this allowed? Does it return an error? If it is allowed, what is supposed to happen?

Test cases are a great way to document and enforce this case.

ethanfrey · 2017-11-01T13:36:09Z

db/cache_db.go

+}
+
+// CacheDB wraps an in-memory cache around an underlying DB.
+type CacheDB struct {


We really should have two caches, a read cache and a write cache... Otherwise, if we query a value, we write that to the db again. Not only is this wasteful use of IO, but with the version tree, this actually changes the hash.

https://github.com/cosmos/cosmos-sdk/blob/develop/state/kvcache.go#L6-L11

Issue:
cosmos/gaia#41
cosmos/cosmos-sdk#271

Fixing so that we have a "dirty" flag.

* Add FSDB * Review fixes from Anton

* Add CacheDB & SimpleMap * Generic memBatch; Fix cLevelDB tests * CacheWrap() for CacheDB and MemDB * Change Iterator to match LeviGo Iterator * Fixes from review * cacheWrapWriteMutex and some race fixes * Use tmlibs/common * NewCWWMutex is exposed. DB can be CacheWrap'd * Remove GetOK, not needed * Fsdb (#72) * Add FSDB * Review fixes from Anton * Review changes * Fixes from review

jaekwon added 3 commits October 23, 2017 14:56

Add CacheDB & SimpleMap

403b23d

Generic memBatch; Fix cLevelDB tests

d8bb20b

CacheWrap() for CacheDB and MemDB

681617d

jaekwon requested review from ebuchman and melekes as code owners October 24, 2017 21:39

ebuchman approved these changes Oct 25, 2017

View reviewed changes

melekes reviewed Oct 25, 2017

View reviewed changes

cloudhead suggested changes Oct 25, 2017

View reviewed changes

ebuchman previously requested changes Oct 25, 2017

View reviewed changes

odeke-em reviewed Oct 26, 2017

View reviewed changes

.gitignore Outdated

@@ -1,4 +1,5 @@

*.swp

Copy link

Contributor

odeke-em Oct 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also do

*.sw[op]

and it's awesome to know that you too @jaekwon use Vim!

odeke-em reviewed Oct 26, 2017

View reviewed changes

db/mem_db.go Outdated

delete(db.db, string(key))

}

// Close is a noop.

Copy link

Contributor

odeke-em Oct 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the doc!

odeke-em reviewed Oct 26, 2017

View reviewed changes

Change Iterator to match LeviGo Iterator

14049a9

jaekwon force-pushed the cachedb branch from d9c040b to 1a763ab Compare October 27, 2017 06:08

melekes reviewed Oct 27, 2017

View reviewed changes

Fixes from review

528bef5

jaekwon force-pushed the cachedb branch from a96dd10 to 528bef5 Compare October 27, 2017 18:39