From ab7506048ec42a4f827b088e5f021dfd96019ed6 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 12:45:13 -0400 Subject: [PATCH 01/14] Remove commented out code --- merkle/clock/heads.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index 7aec511231..fa89a48aac 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -102,14 +102,6 @@ func (hh *heads) Replace(ctx context.Context, h, c cid.Cid, height uint64) error var store ds.Write = hh.store var err error - // batchingDs, batching := store.(ds.Batching) - // if batching { - // store, err = batchingDs.Batch() - // if err != nil { - // return err - // } - // } - err = hh.delete(ctx, store, h) if err != nil { return err @@ -120,12 +112,6 @@ func (hh *heads) Replace(ctx context.Context, h, c cid.Cid, height uint64) error return err } - // if batching { - // err := store.(ds.Batch).Commit() - // if err != nil { - // return err - // } - // } return nil } From a6b582af92762c1d9c36414e18ea2943f0ee665e Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 12:47:08 -0400 Subject: [PATCH 02/14] Remove dead code (Len) --- merkle/clock/clock_test.go | 12 ------------ merkle/clock/heads.go | 5 ----- merkle/clock/heads_test.go | 40 -------------------------------------- 3 files changed, 57 deletions(-) diff --git a/merkle/clock/clock_test.go b/merkle/clock/clock_test.go index 378961aa5d..8a22fc1ece 100644 --- a/merkle/clock/clock_test.go +++ b/merkle/clock/clock_test.go @@ -151,18 +151,6 @@ func TestMerkleClockAddDAGNodeWithHeads(t *testing.T) { ) } - // check if lww state is correct (val is test2) - // check if head/blockstore state is correct (one head, two blocks) - nHeads, err := clk.headset.Len(ctx) - if err != nil { - t.Error("Error getting MerkleClock heads size:", err) - return - } - if nHeads != 1 { - t.Errorf("Incorrect number of heads of current clock state, have %v, want %v", nHeads, 1) - return - } - numBlocks := 0 cids, err := clk.dagstore.AllKeysChan(ctx) if err != nil { diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index fa89a48aac..9267f7ab14 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -86,11 +86,6 @@ func (hh *heads) IsHead(ctx context.Context, c cid.Cid) (bool, uint64, error) { return err == nil, height, err } -func (hh *heads) Len(ctx context.Context) (int, error) { - list, _, err := hh.List(ctx) - return len(list), err -} - // Replace replaces a head with a new CID. func (hh *heads) Replace(ctx context.Context, h, c cid.Cid, height uint64) error { log.Info( diff --git a/merkle/clock/heads_test.go b/merkle/clock/heads_test.go index cb955fdc8d..82dbfc511e 100644 --- a/merkle/clock/heads_test.go +++ b/merkle/clock/heads_test.go @@ -141,46 +141,6 @@ func TestHeadsIsHead(t *testing.T) { } } -func TestHeadsLen(t *testing.T) { - ctx := context.Background() - heads := newHeadSet() - c := newRandomCID() - err := heads.write(ctx, heads.store, c, uint64(1)) - if err != nil { - t.Error("Failed to write to head set:", err) - return - } - - l, err := heads.Len(ctx) - if err != nil { - t.Error("Failed to get head set length:", err) - return - } - - if l != 1 { - t.Errorf("Incorrect length for head set, have %v, want %v", l, 1) - return - } - - c = newRandomCID() - err = heads.write(ctx, heads.store, c, uint64(1)) - if err != nil { - t.Error("Failed to write to head set:", err) - return - } - - l, err = heads.Len(ctx) - if err != nil { - t.Error("Failed to get head set length (second call):", err) - return - } - - if l != 2 { - t.Errorf("Incorrect length for head set, have %v, want %v", l, 2) - return - } -} - func TestHeadsReplaceEmpty(t *testing.T) { ctx := context.Background() heads := newHeadSet() From fd34fb75826cf49454288cbe4762d05aed896817 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 12:49:16 -0400 Subject: [PATCH 03/14] Remove unnessecary var declaration --- merkle/clock/heads.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index 9267f7ab14..3df08f63d6 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -95,9 +95,8 @@ func (hh *heads) Replace(ctx context.Context, h, c cid.Cid, height uint64) error logging.NewKV("CID", c), logging.NewKV("Height", height)) var store ds.Write = hh.store - var err error - err = hh.delete(ctx, store, h) + err := hh.delete(ctx, store, h) if err != nil { return err } From 5fa2b19e780e92685ba8697a8363a8dd16c65167 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 12:51:42 -0400 Subject: [PATCH 04/14] Remove unessecary func param --- merkle/clock/heads.go | 16 ++++++++-------- merkle/clock/heads_test.go | 12 ++++++------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index 3df08f63d6..92ea19ade8 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -60,17 +60,18 @@ func (hh *heads) load(ctx context.Context, c cid.Cid) (uint64, error) { return height, nil } -func (hh *heads) write(ctx context.Context, store ds.Write, c cid.Cid, height uint64) error { +func (hh *heads) write(ctx context.Context, c cid.Cid, height uint64) error { buf := make([]byte, binary.MaxVarintLen64) n := binary.PutUvarint(buf, height) if n == 0 { return errors.New("error encoding height") } - return store.Put(ctx, hh.key(c).ToDS(), buf[0:n]) + + return hh.store.Put(ctx, hh.key(c).ToDS(), buf[0:n]) } -func (hh *heads) delete(ctx context.Context, store ds.Write, c cid.Cid) error { - err := store.Delete(ctx, hh.key(c).ToDS()) +func (hh *heads) delete(ctx context.Context, c cid.Cid) error { + err := hh.store.Delete(ctx, hh.key(c).ToDS()) if errors.Is(err, ds.ErrNotFound) { return nil } @@ -94,14 +95,13 @@ func (hh *heads) Replace(ctx context.Context, h, c cid.Cid, height uint64) error logging.NewKV("Old", h), logging.NewKV("CID", c), logging.NewKV("Height", height)) - var store ds.Write = hh.store - err := hh.delete(ctx, store, h) + err := hh.delete(ctx, h) if err != nil { return err } - err = hh.write(ctx, store, c, height) + err = hh.write(ctx, c, height) if err != nil { return err } @@ -113,7 +113,7 @@ func (hh *heads) Add(ctx context.Context, c cid.Cid, height uint64) error { log.Debug(ctx, "Adding new DAG head", logging.NewKV("CID", c), logging.NewKV("Height", height)) - return hh.write(ctx, hh.store, c, height) + return hh.write(ctx, c, height) } // List returns the list of current heads plus the max height. diff --git a/merkle/clock/heads_test.go b/merkle/clock/heads_test.go index 82dbfc511e..d2f130de3a 100644 --- a/merkle/clock/heads_test.go +++ b/merkle/clock/heads_test.go @@ -62,7 +62,7 @@ func TestHeadsWrite(t *testing.T) { ctx := context.Background() heads := newHeadSet() c := newRandomCID() - err := heads.write(ctx, heads.store, c, uint64(1)) + err := heads.write(ctx, c, uint64(1)) if err != nil { t.Error("Failed to write to head set:", err) return @@ -73,7 +73,7 @@ func TestHeadsLoad(t *testing.T) { ctx := context.Background() heads := newHeadSet() c := newRandomCID() - err := heads.write(ctx, heads.store, c, uint64(1)) + err := heads.write(ctx, c, uint64(1)) if err != nil { t.Error("Failed to write to head set:", err) return @@ -95,13 +95,13 @@ func TestHeadsDelete(t *testing.T) { ctx := context.Background() heads := newHeadSet() c := newRandomCID() - err := heads.write(ctx, heads.store, c, uint64(1)) + err := heads.write(ctx, c, uint64(1)) if err != nil { t.Error("Failed to write to head set:", err) return } - err = heads.delete(ctx, heads.store, c) + err = heads.delete(ctx, c) if err != nil { t.Error("Failed to delete from head set:", err) return @@ -118,7 +118,7 @@ func TestHeadsIsHead(t *testing.T) { ctx := context.Background() heads := newHeadSet() c := newRandomCID() - err := heads.write(ctx, heads.store, c, uint64(1)) + err := heads.write(ctx, c, uint64(1)) if err != nil { t.Error("Failed to write to head set:", err) return @@ -168,7 +168,7 @@ func TestHeadsReplaceNonEmpty(t *testing.T) { ctx := context.Background() heads := newHeadSet() c1 := newRandomCID() - err := heads.write(ctx, heads.store, c1, uint64(1)) + err := heads.write(ctx, c1, uint64(1)) if err != nil { t.Error("Failed to write to head set:", err) return From 354669a3e8da86db28d4368c1490941598cd7140 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 13:00:05 -0400 Subject: [PATCH 05/14] Remove extra Add func Add is a poor name, and adds an extra layer of misdirection. --- merkle/clock/clock.go | 4 ++-- merkle/clock/heads.go | 11 ++--------- merkle/clock/heads_test.go | 16 ++++++++-------- 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/merkle/clock/clock.go b/merkle/clock/clock.go index 31a2edec5e..280cb1248c 100644 --- a/merkle/clock/clock.go +++ b/merkle/clock/clock.go @@ -154,7 +154,7 @@ func (mc *MerkleClock) ProcessNode( } if !hasHeads { // reached the bottom, at a leaf log.Debug(ctx, "No heads found") - err := mc.headset.Add(ctx, root, rootPrio) + err := mc.headset.Write(ctx, root, rootPrio) if err != nil { return nil, errors.Wrap(fmt.Sprintf("error adding head (when reached the bottom) %s ", root), err) } @@ -190,7 +190,7 @@ func (mc *MerkleClock) ProcessNode( // we reached a non-head node in the known tree. // This means our root block is a new head log.Debug(ctx, "Adding head") - err := mc.headset.Add(ctx, root, rootPrio) + err := mc.headset.Write(ctx, root, rootPrio) if err != nil { log.ErrorE( ctx, diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index 92ea19ade8..ad5bdcc7f0 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -60,7 +60,7 @@ func (hh *heads) load(ctx context.Context, c cid.Cid) (uint64, error) { return height, nil } -func (hh *heads) write(ctx context.Context, c cid.Cid, height uint64) error { +func (hh *heads) Write(ctx context.Context, c cid.Cid, height uint64) error { buf := make([]byte, binary.MaxVarintLen64) n := binary.PutUvarint(buf, height) if n == 0 { @@ -101,7 +101,7 @@ func (hh *heads) Replace(ctx context.Context, h, c cid.Cid, height uint64) error return err } - err = hh.write(ctx, c, height) + err = hh.Write(ctx, c, height) if err != nil { return err } @@ -109,13 +109,6 @@ func (hh *heads) Replace(ctx context.Context, h, c cid.Cid, height uint64) error return nil } -func (hh *heads) Add(ctx context.Context, c cid.Cid, height uint64) error { - log.Debug(ctx, "Adding new DAG head", - logging.NewKV("CID", c), - logging.NewKV("Height", height)) - return hh.write(ctx, c, height) -} - // List returns the list of current heads plus the max height. // @todo Document Heads.List function func (hh *heads) List(ctx context.Context) ([]cid.Cid, uint64, error) { diff --git a/merkle/clock/heads_test.go b/merkle/clock/heads_test.go index d2f130de3a..17af496542 100644 --- a/merkle/clock/heads_test.go +++ b/merkle/clock/heads_test.go @@ -62,7 +62,7 @@ func TestHeadsWrite(t *testing.T) { ctx := context.Background() heads := newHeadSet() c := newRandomCID() - err := heads.write(ctx, c, uint64(1)) + err := heads.Write(ctx, c, uint64(1)) if err != nil { t.Error("Failed to write to head set:", err) return @@ -73,7 +73,7 @@ func TestHeadsLoad(t *testing.T) { ctx := context.Background() heads := newHeadSet() c := newRandomCID() - err := heads.write(ctx, c, uint64(1)) + err := heads.Write(ctx, c, uint64(1)) if err != nil { t.Error("Failed to write to head set:", err) return @@ -95,7 +95,7 @@ func TestHeadsDelete(t *testing.T) { ctx := context.Background() heads := newHeadSet() c := newRandomCID() - err := heads.write(ctx, c, uint64(1)) + err := heads.Write(ctx, c, uint64(1)) if err != nil { t.Error("Failed to write to head set:", err) return @@ -118,7 +118,7 @@ func TestHeadsIsHead(t *testing.T) { ctx := context.Background() heads := newHeadSet() c := newRandomCID() - err := heads.write(ctx, c, uint64(1)) + err := heads.Write(ctx, c, uint64(1)) if err != nil { t.Error("Failed to write to head set:", err) return @@ -168,7 +168,7 @@ func TestHeadsReplaceNonEmpty(t *testing.T) { ctx := context.Background() heads := newHeadSet() c1 := newRandomCID() - err := heads.write(ctx, c1, uint64(1)) + err := heads.Write(ctx, c1, uint64(1)) if err != nil { t.Error("Failed to write to head set:", err) return @@ -201,7 +201,7 @@ func TestHeadsAdd(t *testing.T) { ctx := context.Background() heads := newHeadSet() c1 := newRandomCID() - err := heads.Add(ctx, c1, uint64(1)) + err := heads.Write(ctx, c1, uint64(1)) if err != nil { t.Error("Failed to Add element to head set:", err) return @@ -213,8 +213,8 @@ func TestHeaddsList(t *testing.T) { heads := newHeadSet() c1 := newRandomCID() c2 := newRandomCID() - heads.Add(ctx, c1, uint64(1)) - heads.Add(ctx, c2, uint64(2)) + heads.Write(ctx, c1, uint64(1)) + heads.Write(ctx, c2, uint64(2)) list, h, err := heads.List(ctx) if err != nil { From 0ba9edd4872d0c3691fdca59c237e25ec3bef352 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 13:05:47 -0400 Subject: [PATCH 06/14] Remove extra delete func Is only called from here, and the caller to this function knows that this row exists, so we will never actually hit the isNotFound error allowing it to be safely dropped. --- merkle/clock/heads.go | 10 +--------- merkle/clock/heads_test.go | 25 ------------------------- 2 files changed, 1 insertion(+), 34 deletions(-) diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index ad5bdcc7f0..855467165a 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -70,14 +70,6 @@ func (hh *heads) Write(ctx context.Context, c cid.Cid, height uint64) error { return hh.store.Put(ctx, hh.key(c).ToDS(), buf[0:n]) } -func (hh *heads) delete(ctx context.Context, c cid.Cid) error { - err := hh.store.Delete(ctx, hh.key(c).ToDS()) - if errors.Is(err, ds.ErrNotFound) { - return nil - } - return err -} - // IsHead returns if a given cid is among the current heads. func (hh *heads) IsHead(ctx context.Context, c cid.Cid) (bool, uint64, error) { height, err := hh.load(ctx, c) @@ -96,7 +88,7 @@ func (hh *heads) Replace(ctx context.Context, h, c cid.Cid, height uint64) error logging.NewKV("CID", c), logging.NewKV("Height", height)) - err := hh.delete(ctx, h) + err := hh.store.Delete(ctx, hh.key(h).ToDS()) if err != nil { return err } diff --git a/merkle/clock/heads_test.go b/merkle/clock/heads_test.go index 17af496542..d9e3680428 100644 --- a/merkle/clock/heads_test.go +++ b/merkle/clock/heads_test.go @@ -20,12 +20,10 @@ import ( "testing" "github.com/ipfs/go-cid" - ds "github.com/ipfs/go-datastore" mh "github.com/multiformats/go-multihash" "github.com/sourcenetwork/defradb/core" "github.com/sourcenetwork/defradb/datastore" - "github.com/sourcenetwork/defradb/errors" ) func newRandomCID() cid.Cid { @@ -91,29 +89,6 @@ func TestHeadsLoad(t *testing.T) { } } -func TestHeadsDelete(t *testing.T) { - ctx := context.Background() - heads := newHeadSet() - c := newRandomCID() - err := heads.Write(ctx, c, uint64(1)) - if err != nil { - t.Error("Failed to write to head set:", err) - return - } - - err = heads.delete(ctx, c) - if err != nil { - t.Error("Failed to delete from head set:", err) - return - } - - _, err = heads.load(ctx, c) - if !errors.Is(err, ds.ErrNotFound) { - t.Error("failed to delete from head set, value still set") - return - } -} - func TestHeadsIsHead(t *testing.T) { ctx := context.Background() heads := newHeadSet() From 5cdb750a6b116ae2697735f7501e253fc1c2f397 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 13:12:54 -0400 Subject: [PATCH 07/14] Rename func params Old names unhelpful and the distinction was easily missed resulting in a bug (caught by tests). --- merkle/clock/heads.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index 855467165a..4774366368 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -80,20 +80,20 @@ func (hh *heads) IsHead(ctx context.Context, c cid.Cid) (bool, uint64, error) { } // Replace replaces a head with a new CID. -func (hh *heads) Replace(ctx context.Context, h, c cid.Cid, height uint64) error { +func (hh *heads) Replace(ctx context.Context, old cid.Cid, new cid.Cid, height uint64) error { log.Info( ctx, "Replacing DAG head", - logging.NewKV("Old", h), - logging.NewKV("CID", c), + logging.NewKV("Old", old), + logging.NewKV("CID", new), logging.NewKV("Height", height)) - err := hh.store.Delete(ctx, hh.key(h).ToDS()) + err := hh.store.Delete(ctx, hh.key(old).ToDS()) if err != nil { return err } - err = hh.Write(ctx, c, height) + err = hh.Write(ctx, new, height) if err != nil { return err } From 9fb412d88f975f207c6acdfc50dc2feddbb298f9 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 13:17:41 -0400 Subject: [PATCH 08/14] Unfactor IsHead Is only called once, and the unfactoring allows for future refactorings --- merkle/clock/heads.go | 20 ++++++----------- merkle/clock/heads_test.go | 44 -------------------------------------- 2 files changed, 7 insertions(+), 57 deletions(-) diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index 4774366368..539fec75a7 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -48,18 +48,6 @@ func (hh *heads) key(c cid.Cid) core.HeadStoreKey { return hh.namespace.WithCid(c) } -func (hh *heads) load(ctx context.Context, c cid.Cid) (uint64, error) { - v, err := hh.store.Get(ctx, hh.key(c).ToDS()) - if err != nil { - return 0, err - } - height, n := binary.Uvarint(v) - if n <= 0 { - return 0, errors.New("error decoding height") - } - return height, nil -} - func (hh *heads) Write(ctx context.Context, c cid.Cid, height uint64) error { buf := make([]byte, binary.MaxVarintLen64) n := binary.PutUvarint(buf, height) @@ -72,10 +60,16 @@ func (hh *heads) Write(ctx context.Context, c cid.Cid, height uint64) error { // IsHead returns if a given cid is among the current heads. func (hh *heads) IsHead(ctx context.Context, c cid.Cid) (bool, uint64, error) { - height, err := hh.load(ctx, c) + v, err := hh.store.Get(ctx, hh.key(c).ToDS()) if errors.Is(err, ds.ErrNotFound) { return false, 0, nil } + + height, n := binary.Uvarint(v) + if n <= 0 { + return false, 0, errors.New("error decoding height") + } + return err == nil, height, err } diff --git a/merkle/clock/heads_test.go b/merkle/clock/heads_test.go index d9e3680428..55eb165846 100644 --- a/merkle/clock/heads_test.go +++ b/merkle/clock/heads_test.go @@ -67,28 +67,6 @@ func TestHeadsWrite(t *testing.T) { } } -func TestHeadsLoad(t *testing.T) { - ctx := context.Background() - heads := newHeadSet() - c := newRandomCID() - err := heads.Write(ctx, c, uint64(1)) - if err != nil { - t.Error("Failed to write to head set:", err) - return - } - - h, err := heads.load(ctx, c) - if err != nil { - t.Error("failed to load from head set:", err) - return - } - - if h != uint64(1) { - t.Errorf("Incorrect value from head set load(), have %v, want %v", h, uint64(1)) - return - } -} - func TestHeadsIsHead(t *testing.T) { ctx := context.Background() heads := newHeadSet() @@ -126,17 +104,6 @@ func TestHeadsReplaceEmpty(t *testing.T) { t.Error("Failed to Replace items in head set:", err) return } - - h, err := heads.load(ctx, c2) - if err != nil { - t.Error("Failed to load items in head set:", err) - return - } - - if h != uint64(3) { - t.Errorf("Invalid value for replaced head element, have %v, want %v", h, uint64(3)) - return - } } func TestHeadsReplaceNonEmpty(t *testing.T) { @@ -155,17 +122,6 @@ func TestHeadsReplaceNonEmpty(t *testing.T) { t.Error("Failed to Replace items in head set:", err) return } - - h, err := heads.load(ctx, c2) - if err != nil { - t.Error("Failed to load items in head set:", err) - return - } - - if h != uint64(3) { - t.Errorf("Invalid value for replaced head element, have %v, want %v", h, uint64(3)) - return - } } // this test is largely unneeded from a functional point of view From 0f75bd771103790720359348fe5a76bb9661ba83 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 13:20:10 -0400 Subject: [PATCH 09/14] Remove unused return param --- merkle/clock/clock.go | 2 +- merkle/clock/heads.go | 13 ++++--------- merkle/clock/heads_test.go | 27 --------------------------- 3 files changed, 5 insertions(+), 37 deletions(-) diff --git a/merkle/clock/clock.go b/merkle/clock/clock.go index 280cb1248c..8817f43177 100644 --- a/merkle/clock/clock.go +++ b/merkle/clock/clock.go @@ -165,7 +165,7 @@ func (mc *MerkleClock) ProcessNode( for _, l := range links { child := l.Cid log.Debug(ctx, "Scanning for replacement heads", logging.NewKV("Child", child)) - isHead, _, err := mc.headset.IsHead(ctx, child) + isHead, err := mc.headset.IsHead(ctx, child) if err != nil { return nil, errors.Wrap(fmt.Sprintf("error checking if %s is head ", child), err) } diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index 539fec75a7..4e316aa13e 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -59,18 +59,13 @@ func (hh *heads) Write(ctx context.Context, c cid.Cid, height uint64) error { } // IsHead returns if a given cid is among the current heads. -func (hh *heads) IsHead(ctx context.Context, c cid.Cid) (bool, uint64, error) { - v, err := hh.store.Get(ctx, hh.key(c).ToDS()) +func (hh *heads) IsHead(ctx context.Context, c cid.Cid) (bool, error) { + _, err := hh.store.Get(ctx, hh.key(c).ToDS()) if errors.Is(err, ds.ErrNotFound) { - return false, 0, nil + return false, nil } - height, n := binary.Uvarint(v) - if n <= 0 { - return false, 0, errors.New("error decoding height") - } - - return err == nil, height, err + return err == nil, err } // Replace replaces a head with a new CID. diff --git a/merkle/clock/heads_test.go b/merkle/clock/heads_test.go index 55eb165846..3d1e5c00cc 100644 --- a/merkle/clock/heads_test.go +++ b/merkle/clock/heads_test.go @@ -67,33 +67,6 @@ func TestHeadsWrite(t *testing.T) { } } -func TestHeadsIsHead(t *testing.T) { - ctx := context.Background() - heads := newHeadSet() - c := newRandomCID() - err := heads.Write(ctx, c, uint64(1)) - if err != nil { - t.Error("Failed to write to head set:", err) - return - } - - ishead, h, err := heads.IsHead(ctx, c) - if err != nil { - t.Error("Failedd to check isHead:", err) - return - } - - if ishead == false { - t.Error("Expected isHead to return true, instead false") - return - } - - if h != uint64(1) { - t.Errorf("Incorrect height value from isHead, have %v, want %v", h, uint64(1)) - return - } -} - func TestHeadsReplaceEmpty(t *testing.T) { ctx := context.Background() heads := newHeadSet() From 39eb7c6298098482116f2306951e5e65bd270b63 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 13:23:04 -0400 Subject: [PATCH 10/14] Use Has instead of Get+IsError --- merkle/clock/heads.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index 4e316aa13e..fe832ef11b 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -17,7 +17,6 @@ import ( "sort" cid "github.com/ipfs/go-cid" - ds "github.com/ipfs/go-datastore" "github.com/ipfs/go-datastore/query" "github.com/sourcenetwork/defradb/core" @@ -60,12 +59,7 @@ func (hh *heads) Write(ctx context.Context, c cid.Cid, height uint64) error { // IsHead returns if a given cid is among the current heads. func (hh *heads) IsHead(ctx context.Context, c cid.Cid) (bool, error) { - _, err := hh.store.Get(ctx, hh.key(c).ToDS()) - if errors.Is(err, ds.ErrNotFound) { - return false, nil - } - - return err == nil, err + return hh.store.Has(ctx, hh.key(c).ToDS()) } // Replace replaces a head with a new CID. From ef2aa7efd95307cb7aed96978e04840914a3fa65 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 13:25:49 -0400 Subject: [PATCH 11/14] Remove private constructor --- merkle/clock/clock.go | 2 +- merkle/clock/heads.go | 4 ---- merkle/clock/heads_test.go | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/merkle/clock/clock.go b/merkle/clock/clock.go index 8817f43177..aa07e79603 100644 --- a/merkle/clock/clock.go +++ b/merkle/clock/clock.go @@ -46,7 +46,7 @@ func NewMerkleClock( return &MerkleClock{ headstore: headstore, dagstore: dagstore, - headset: newHeadset(headstore, namespace), + headset: NewHeadSet(headstore, namespace), crdt: crdt, } } diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index fe832ef11b..0cca4284ae 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -32,10 +32,6 @@ type heads struct { } func NewHeadSet(store datastore.DSReaderWriter, namespace core.HeadStoreKey) *heads { - return newHeadset(store, namespace) -} - -func newHeadset(store datastore.DSReaderWriter, namespace core.HeadStoreKey) *heads { return &heads{ store: store, namespace: namespace, diff --git a/merkle/clock/heads_test.go b/merkle/clock/heads_test.go index 3d1e5c00cc..c9c6212c5c 100644 --- a/merkle/clock/heads_test.go +++ b/merkle/clock/heads_test.go @@ -50,7 +50,7 @@ func newRandomCID() cid.Cid { func newHeadSet() *heads { s := newDS() - return newHeadset( + return NewHeadSet( datastore.AsDSReaderWriter(s), core.HeadStoreKey{}.WithDocKey("mydockey").WithFieldId("1"), ) From d4c715987834490eaebe9f6a3ea4da4c4d79d8af Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 13:30:21 -0400 Subject: [PATCH 12/14] Remove incorrect error Error would have been way up the callstack where ever height was declared, and it is on those funcs to make sure it fits whatever constraints they may have. --- merkle/clock/heads.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index 0cca4284ae..9a10efa77d 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -46,9 +46,6 @@ func (hh *heads) key(c cid.Cid) core.HeadStoreKey { func (hh *heads) Write(ctx context.Context, c cid.Cid, height uint64) error { buf := make([]byte, binary.MaxVarintLen64) n := binary.PutUvarint(buf, height) - if n == 0 { - return errors.New("error encoding height") - } return hh.store.Put(ctx, hh.key(c).ToDS(), buf[0:n]) } From 79bfef994e217020333d413f594343d406908d45 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 13:57:03 -0400 Subject: [PATCH 13/14] Remove unhelpful comment Is also potentially misleading as the cid's location in the key is dependent on the index format, not the code in this file/package --- merkle/clock/heads.go | 1 - 1 file changed, 1 deletion(-) diff --git a/merkle/clock/heads.go b/merkle/clock/heads.go index 9a10efa77d..afe388f9cf 100644 --- a/merkle/clock/heads.go +++ b/merkle/clock/heads.go @@ -39,7 +39,6 @@ func NewHeadSet(store datastore.DSReaderWriter, namespace core.HeadStoreKey) *he } func (hh *heads) key(c cid.Cid) core.HeadStoreKey { - // // return hh.namespace.WithCid(c) } From 50b7193a918320123ee9c0c1045dcd7843fdd555 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 26 Oct 2022 11:27:21 -0400 Subject: [PATCH 14/14] Remove cid-based shortcut This optimization should be handled by the index, not the scan code. It also produces undesirable errors which have been corrected in this commit. --- planner/commit.go | 16 ---------------- tests/integration/query/commits/with_cid_test.go | 8 ++------ 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/planner/commit.go b/planner/commit.go index d8ac18dcf5..7e971129ae 100644 --- a/planner/commit.go +++ b/planner/commit.go @@ -165,22 +165,6 @@ func (n *dagScanNode) Next() (bool, error) { if len(n.queuedCids) > 0 { currentCid = n.queuedCids[0] n.queuedCids = n.queuedCids[1:(len(n.queuedCids))] - } else if n.parsed.Cid.HasValue() && !n.parsed.DocKey.HasValue() { - if n.visitedNodes[n.parsed.Cid.Value()] { - // If the requested cid has been visited, we are done and should return false - return false, nil - } - - cid, err := cid.Decode(n.parsed.Cid.Value()) - if err != nil { - return false, err - } - - if hasCid, err := store.Has(n.p.ctx, cid); !hasCid || err != nil { - return false, err - } - - currentCid = &cid } else { cid, err := n.fetcher.FetchNext() if err != nil || cid == nil { diff --git a/tests/integration/query/commits/with_cid_test.go b/tests/integration/query/commits/with_cid_test.go index 0ac6b53815..71e30c17c9 100644 --- a/tests/integration/query/commits/with_cid_test.go +++ b/tests/integration/query/commits/with_cid_test.go @@ -82,8 +82,6 @@ func TestQueryCommitsWithCidForFieldCommit(t *testing.T) { executeTestCase(t, test) } -// This test is for documentation reasons only. This is not -// desired behaviour (error message could be better, or empty result). func TestQueryCommitsWithInvalidCid(t *testing.T) { test := testUtils.QueryTestCase{ Description: "query for a single block by invalid CID", @@ -102,14 +100,12 @@ func TestQueryCommitsWithInvalidCid(t *testing.T) { }`, }, }, - ExpectedError: "encoding/hex: invalid byte:", + Results: []map[string]any{}, } executeTestCase(t, test) } -// This test is for documentation reasons only. This is not -// desired behaviour (error message could be better, or empty result). func TestQueryCommitsWithInvalidShortCid(t *testing.T) { test := testUtils.QueryTestCase{ Description: "query for a single block by invalid, short CID", @@ -128,7 +124,7 @@ func TestQueryCommitsWithInvalidShortCid(t *testing.T) { }`, }, }, - ExpectedError: "length greater than remaining number of bytes in buffer", + Results: []map[string]any{}, } executeTestCase(t, test)