From 7cfcab8d4a6471bac6d0e2d6ca7b4c3835358f45 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Mon, 17 Jul 2023 16:21:07 -0400 Subject: [PATCH 1/2] `merkledb` / `sync` -- remove TODOs (#1718) --- x/merkledb/db.go | 1 - x/sync/manager.go | 5 ----- x/sync/network_client.go | 10 ---------- 3 files changed, 16 deletions(-) diff --git a/x/merkledb/db.go b/x/merkledb/db.go index 4616e51a871..aee28de2ec8 100644 --- a/x/merkledb/db.go +++ b/x/merkledb/db.go @@ -566,7 +566,6 @@ func (db *merkleDB) GetChangeProof( return i.Compare(j) < 0 }) - // TODO: sync.pool these buffers result.KeyChanges = make([]KeyChange, 0, len(changedKeys)) for _, key := range changedKeys { diff --git a/x/sync/manager.go b/x/sync/manager.go index 992451312f0..c239a42b8d4 100644 --- a/x/sync/manager.go +++ b/x/sync/manager.go @@ -55,7 +55,6 @@ type workItem struct { localRootID ids.ID } -// TODO danlaine look into using a sync.Pool for workItems func newWorkItem(localRootID ids.ID, start, end []byte, priority priority) *workItem { return &workItem{ localRootID: localRootID, @@ -190,10 +189,6 @@ func (m *Manager) sync(ctx context.Context) { default: m.processingWorkItems++ work := m.unprocessedWork.GetWork() - // TODO danlaine: We won't release [m.workLock] until - // we've started a goroutine for each available work item. - // We can't apply proofs we receive until we release [m.workLock]. - // Is this OK? Is it possible we end up with too many goroutines? go m.doWork(ctx, work) } } diff --git a/x/sync/network_client.go b/x/sync/network_client.go index 5b312882ff9..8354b041863 100644 --- a/x/sync/network_client.go +++ b/x/sync/network_client.go @@ -306,16 +306,6 @@ func (c *networkClient) Disconnected(_ context.Context, nodeID ids.NodeID) error return nil } -// Shutdown disconnects all peers -func (c *networkClient) Shutdown() { - c.lock.Lock() - defer c.lock.Unlock() - - // reset peers - // TODO danlaine: should we call [Disconnected] on each peer? - c.peers = newPeerTracker(c.log) -} - func (c *networkClient) TrackBandwidth(nodeID ids.NodeID, bandwidth float64) { c.lock.Lock() defer c.lock.Unlock() From 7ac713d8e30230dde17ecfb624c5bdd644d570f1 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Mon, 17 Jul 2023 16:48:31 -0400 Subject: [PATCH 2/2] Remove cache TODOs (#1721) --- x/merkledb/db.go | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/x/merkledb/db.go b/x/merkledb/db.go index aee28de2ec8..1a1ddef6981 100644 --- a/x/merkledb/db.go +++ b/x/merkledb/db.go @@ -148,6 +148,9 @@ type merkleDB struct { metadataDB database.Database // If a value is nil, the corresponding key isn't in the trie. + // Note that a call to Put may cause a node to be evicted + // from the cache, which will call [OnEviction]. + // A non-nil error returned from Put is considered fatal. nodeCache onEvictCache[path, *node] onEvictionErr utils.Atomic[error] evictionBatchSize int @@ -923,7 +926,7 @@ func (db *merkleDB) commitChanges(ctx context.Context, trieToCommit *trieView) e db.root = rootChange.after for key, nodeChange := range changes.nodes { - if err := db.putNodeInCache(key, nodeChange.after); err != nil { + if err := db.nodeCache.Put(key, nodeChange.after); err != nil { return err } } @@ -1233,7 +1236,7 @@ func (db *merkleDB) getNode(key path) (*node, error) { return db.root, nil } - if n, isCached := db.getNodeInCache(key); isCached { + if n, isCached := db.nodeCache.Get(key); isCached { db.metrics.DBNodeCacheHit() if n == nil { return nil, database.ErrNotFound @@ -1247,7 +1250,7 @@ func (db *merkleDB) getNode(key path) (*node, error) { if err != nil { if err == database.ErrNotFound { // Cache the miss. - if err := db.putNodeInCache(key, nil); err != nil { + if err := db.nodeCache.Put(key, nil); err != nil { return nil, err } } @@ -1259,7 +1262,7 @@ func (db *merkleDB) getNode(key path) (*node, error) { return nil, err } - err = db.putNodeInCache(key, node) + err = db.nodeCache.Put(key, node) return node, err } @@ -1340,19 +1343,3 @@ func (db *merkleDB) prepareRangeProofView(start []byte, proof *RangeProof) (*tri } return view, nil } - -// Non-nil error is fatal -- [db] will close. -func (db *merkleDB) putNodeInCache(key path, n *node) error { - // TODO Cache metrics - // Note that this may cause a node to be evicted from the cache, - // which will call [OnEviction]. - return db.nodeCache.Put(key, n) -} - -func (db *merkleDB) getNodeInCache(key path) (*node, bool) { - // TODO Cache metrics - if node, ok := db.nodeCache.Get(key); ok { - return node, true - } - return nil, false -}