Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leveldb #322

Merged
merged 27 commits into from Dec 17, 2018
Merged

Leveldb #322

merged 27 commits into from Dec 17, 2018

Conversation

@almogdepaz
Copy link
Contributor

@almogdepaz almogdepaz commented Dec 12, 2018

add level db to our mesh for persistence

almogdepaz added 18 commits Dec 9, 2018
fix tests
added new block handler
error handling
# Conflicts:
#	vendor/github.com/grpc-ecosystem/grpc-gateway/utilities/pattern.go
@almogdepaz almogdepaz requested a review from zalmen Dec 13, 2018
almogdepaz added 2 commits Dec 13, 2018
remove MashDB interface (not needed)
db.db.Close()
}

func (db LevelDB) Put(key, value []byte) error {

This comment has been minimized.

@antonlerner

antonlerner Dec 13, 2018
Member

in my opnion receiver should be of pointer type

This comment has been minimized.

@almogdepaz

almogdepaz Dec 16, 2018
Author Contributor

LevelDB holds only pointers

defer m.lMutex.Unlock()
count := LayerID(m.LatestIrreversible())
if count > layer.Index() {
log.Debug("can't add layer ", layer.Index(), "(already exists)")

This comment has been minimized.

@antonlerner

antonlerner Dec 13, 2018
Member

what if layers are not ordered?

func (m *mesh) SetLatestKnownLayer(idx uint32) {
defer m.lkMutex.Unlock()
m.lkMutex.Lock()
if idx > m.latestKnownLayer {

This comment has been minimized.

@antonlerner

antonlerner Dec 13, 2018
Member

using atomic here will simplify code as well

return err
}
m.SetLatestKnownLayer(uint32(block.Layer()))
m.tortoise.HandleLateBlock(block) //todo should be thread safe?

This comment has been minimized.

@antonlerner

antonlerner Dec 13, 2018
Member

yes, but lets discuss this


func (m *mesh) Close() {
log.Debug("closing mDB")
m.mDB.Close()

This comment has been minimized.

@antonlerner

antonlerner Dec 13, 2018
Member

should we keep a closed flag? otherwise what are the consequences of closing?

bytes, err := blockAsBytes(*b)
if err != nil {
log.Error("problem serializing block ", b.ID(), err)
delete(ids, b.ID()) //remove failed block from layer

This comment has been minimized.

@antonlerner

antonlerner Dec 13, 2018
Member

This error is critical and should not be supressed

This comment has been minimized.

@almogdepaz

almogdepaz Dec 16, 2018
Author Contributor

i don't think we should stop adding the whole layer because of a failed block


err = m.blocks.Put(b.ID().ToBytes(), bytes)
if err != nil {
log.Error("could not add block ", b.ID(), " to database ", err)

This comment has been minimized.

@antonlerner

antonlerner Dec 13, 2018
Member

same here

layerLock, found := m.layerLocks[index]
if !found {
layerLock = sync.Mutex{}
m.layerLocks[index] = layerLock

This comment has been minimized.

@antonlerner

antonlerner Dec 13, 2018
Member

leak - there is no mechanism for removing unused locks

@@ -198,7 +202,7 @@ func (s *Syncer) getLayerBlockIDs(index uint32) chan uint32 {
m[string(hash)] = p
}

idSet := make(map[uint32]bool, 300) //todo move this to config
idSet := make(map[uint64]bool, 300) //todo move this to config

This comment has been minimized.

@antonlerner

antonlerner Dec 13, 2018
Member

please move to config or at least name it

This comment has been minimized.

@almogdepaz

almogdepaz Dec 16, 2018
Author Contributor

unnamed and not labeled because this goes to protobuff serialisation right now and protobuff dosent handle that very well

@@ -213,24 +217,28 @@ func (s *Syncer) getLayerBlockIDs(index uint32) chan uint32 {
}
}

res := make(chan uint32, len(idSet))
res := make(chan mesh.BlockID, len(idSet))

This comment has been minimized.

@antonlerner

antonlerner Dec 13, 2018
Member

typedef chan if using it as a queue

almogdepaz added 7 commits Dec 16, 2018
# Conflicts:
#	sync/sync.go
#	sync/sync_test.go
add todo
use labels better
@almogdepaz almogdepaz merged commit 5912efb into develop Dec 17, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@y0sher y0sher deleted the leveldb branch Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants