ipfs / go-ds-crdt

A distributed go-datastore implementation using Merkle-CRDTs.
Other
397 stars 43 forks source link

Intermittent test failer with dstest.SubtestBasicSync #50

Closed xiegeo closed 4 years ago

xiegeo commented 4 years ago

After running the full test overnight, I encounter a failer in the SubtestBasicSync of TestDatastoreSuite, with the error message datastore: key not found.

The fastest way to reproduce this error is replace the line dstest.SubtestAll(t, replicas[0]) and all checks after it with dstest.SubtestBasicSync(t, replicas[0]). Then run go test -count=10. The test then fails a few times.

welcome[bot] commented 4 years ago

Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review. In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment. Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

Finally, remember to use https://discuss.ipfs.io if you just need general support.

hsanjuan commented 4 years ago

I cannot reproduce... can you paste logs?

Given that the tests run with a map datastore in which Sync() is a no-op, and given what SubtestBasicSync does, I also cannot readily explain such an error.

xiegeo commented 4 years ago

@hsanjuan Thank you for looking into it. Now that I double checked, the error actually happens in the clearDs function, which was copied from dstest. Sorry for making that mistake.

Here is my modifications to TestDatastoreSuite

func TestDatastoreSuite(t *testing.T) {
    numReplicasOld := numReplicas
    numReplicas = 1
    defer func() {
        numReplicas = numReplicasOld
    }()
    opts := DefaultOptions()
    opts.MaxBatchDeltaSize = 200 * 1024 * 1024 // 200 MB
    replicas, closeReplicas := makeReplicas(t, opts)
    defer closeReplicas()
    //dstest.SubtestAll(t, replicas[0])
    dstest.SubtestBasicSync(t, replicas[0])
    clearDs(t, replicas[0])
    time.Sleep(time.Second)

    for _, r := range replicas {
        q := query.Query{KeysOnly: true}
        results, err := r.Query(q)
        if err != nil {
            t.Fatal(err)
        }
        defer results.Close()
        rest, err := results.Rest()
        if err != nil {
            t.Fatal(err)
        }
        if len(rest) != 0 {
            t.Error("all elements in the suite should be gone")
        }
    }
}

func clearDs(t *testing.T, d ds.Datastore) {
    q, err := d.Query(query.Query{KeysOnly: true})
    if err != nil {
        t.Fatal(err)
    }
    res, err := q.Rest()
    if err != nil {
        t.Fatal(err)
    }
    for _, r := range res {
        if err := d.Delete(ds.RawKey(r.Key)); err != nil {
            t.Fatal(err) //line 376 is here
        }
    }
}

Here is a few runs of the test

xiege@LAPTOP-NPU545OC MINGW64 ~/go/src/github.com/ipfs/go-ds-crdt (master)
$ go test -count=1 -v -run="Suite"
=== RUN   TestDatastoreSuite
--- PASS: TestDatastoreSuite (1.00s)
PASS
ok      github.com/ipfs/go-ds-crdt      1.255s

xiege@LAPTOP-NPU545OC MINGW64 ~/go/src/github.com/ipfs/go-ds-crdt (master)
$ go test -count=1 -v -run="Suite"
=== RUN   TestDatastoreSuite
    TestDatastoreSuite: crdt_test.go:376: datastore: key not found
--- FAIL: TestDatastoreSuite (0.01s)
FAIL
exit status 1
FAIL    github.com/ipfs/go-ds-crdt      0.293s

xiege@LAPTOP-NPU545OC MINGW64 ~/go/src/github.com/ipfs/go-ds-crdt (master)
$ go test -count=1 -v -run="Suite"
=== RUN   TestDatastoreSuite
    TestDatastoreSuite: crdt_test.go:376: datastore: key not found
--- FAIL: TestDatastoreSuite (0.01s)
FAIL
exit status 1
FAIL    github.com/ipfs/go-ds-crdt      0.247s

xiege@LAPTOP-NPU545OC MINGW64 ~/go/src/github.com/ipfs/go-ds-crdt (master)
$ go test -count=1 -v -run="Suite"
=== RUN   TestDatastoreSuite
    TestDatastoreSuite: crdt_test.go:376: datastore: key not found
--- FAIL: TestDatastoreSuite (0.01s)
FAIL
exit status 1
FAIL    github.com/ipfs/go-ds-crdt      0.248s

xiege@LAPTOP-NPU545OC MINGW64 ~/go/src/github.com/ipfs/go-ds-crdt (master)
$ go test -count=1 -v -run="Suite"
=== RUN   TestDatastoreSuite
--- PASS: TestDatastoreSuite (1.00s)
PASS
ok      github.com/ipfs/go-ds-crdt      1.226s
hsanjuan commented 4 years ago

Ok, this is a bug in the Rmv set operation, which matches too many elements (deleting /prefix will also remove /prefix/sub). Thanks for finding it! I will send a fix in a bit.

hsanjuan commented 4 years ago

@xiegeo can you double check with https://github.com/ipfs/go-ds-crdt/pull/55/commits/1f95e63346456565b6d775def849400341523afc that this does not happen anymore?

xiegeo commented 4 years ago

@hsanjuan Yes that fixed it. Thank you very much. I'm going to run the full test suite later overnight to make sure there are no regressions.