Skip to content

Commit

Permalink
fix: Fix invalid (badger) datastore state (#1685)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves #1672

## Description

Fixes invalid datastore state where somehow the iterator is not closed
properly, even though Close is called. I am guessing that something is
getting GC'd before it can be closed, as closing it immediately after
use appears to remove the issue.

Bug discovered testing with badger-file, but I cannot be sure where the
GC'd object is, and so it could be within the ipfs code, which could
mean other datastore implementations were exposed to this.

Fix is commit `Close iterator on spanDone` *only*, the others are small
improvements to the codebase I found along the way.
  • Loading branch information
AndrewSisley authored Jul 22, 2023
1 parent eda250a commit 6d896ba
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 21 deletions.
1 change: 1 addition & 0 deletions db/fetcher/encoded_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func (encdoc *encodedDocument) Reset() {
}
encdoc.filterSet = nil
encdoc.selectSet = nil
encdoc.schemaVersionID = ""
}

// Decode returns a properly decoded document object
Expand Down
28 changes: 14 additions & 14 deletions db/fetcher/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,10 @@ func (df *DocumentFetcher) nextKey(ctx context.Context, seekNext bool) (spanDone

df.kvEnd = spanDone
if df.kvEnd {
err = df.kvResultsIter.Close()
if err != nil {
return false, false, err
}
moreSpans, err := df.startNextSpan(ctx)
if err != nil {
return false, false, err
Expand Down Expand Up @@ -656,22 +660,18 @@ func (df *DocumentFetcher) FetchNextDoc(

// Close closes the DocumentFetcher.
func (df *DocumentFetcher) Close() error {
if df.kvIter == nil {
return nil
}

err := df.kvIter.Close()
if err != nil {
return err
}

if df.kvResultsIter == nil {
return nil
if df.kvIter != nil {
err := df.kvIter.Close()
if err != nil {
return err
}
}

err = df.kvResultsIter.Close()
if err != nil {
return err
if df.kvResultsIter != nil {
err := df.kvResultsIter.Close()
if err != nil {
return err
}
}

if df.deletedDocFetcher != nil {
Expand Down
3 changes: 0 additions & 3 deletions planner/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ type scanNode struct {
filter *mapper.Filter
slct *mapper.Select

scanInitialized bool

fetcher fetcher.Fetcher

execInfo scanExecInfo
Expand Down Expand Up @@ -152,7 +150,6 @@ func (n *scanNode) initScan() error {
return err
}

n.scanInitialized = true
return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,6 @@ func TestQueryOneToOneWithGroupRelatedID(t *testing.T) {
testUtils.ExecuteTestCase(t, []string{"Book", "Author"}, test)
}

/* This test is temporarily disabled due to:
https://github.com/sourcenetwork/defradb/issues/1672
// This test documents unwanted behaviour, see:
// https://github.com/sourcenetwork/defradb/issues/1654
func TestQueryOneToOneWithGroupRelatedIDFromSecondary(t *testing.T) {
Expand Down Expand Up @@ -177,4 +174,3 @@ func TestQueryOneToOneWithGroupRelatedIDFromSecondary(t *testing.T) {

testUtils.ExecuteTestCase(t, []string{"Book", "Author"}, test)
}
*/

0 comments on commit 6d896ba

Please sign in to comment.