aerospike / aerospike-client-go

Aerospike Client Go
Apache License 2.0
430 stars 198 forks source link

Problem with `ScanAll` and invalid namespace #308

Closed bignacio closed 4 years ago

bignacio commented 4 years ago

Hi all, I run into an issue where doing a ScanAll on an invalid namespace can return a record set that is not empty but it has nil data

While I understand that should not be a problem in normal circumstances, it could be an indication of an underlying concurrency problem.

here's a way to reproduce it Using aerospike client v2.9.0

package main

import (
    "fmt"
    "time"

    as "github.com/aerospike/aerospike-client-go"
)

func main() {
    cp := as.NewClientPolicy()

    asc, err := as.NewClientWithPolicy(cp, "127.0.0.1", 3000)

    if err != nil {
        panic(err)
    }

    sp := as.NewScanPolicy()
    recordSet, err := asc.ScanAll(sp, "invalid-namespace", "valid-set", "valid-bin")

    if err != nil {
        panic(err)
    }

    // sometimes it works with sleep
    time.Sleep(time.Second * 2)
    for res := range recordSet.Results() {
        r := res.Record
        if r == nil {
            panic("record is nil")
        }
        fmt.Println(r.Bins)
    }

    recordSet, err = asc.ScanAll(sp, "invalid-namespace", "valid-set", "valid-bin")
    if err != nil {
        panic(err)
    }

    // always fails without sleep
    for res := range recordSet.Results() {
        r := res.Record
        if r == nil {
            panic("record is nil")
        }
        fmt.Println(r.Bins)
    }

    fmt.Println("test done")
}

note that the namespace given to ScanAll must not exist or have not been initialized (this problem is also seen when aerospike is initializing, just not very frequent.

If there's time long enough between the call scan and iterating the recordset results, then the problem is not visible. Here I tried to simulate that with a long sleep.

khaf commented 4 years ago

It's not returning a record, it's returning a Result. It has a Record and an Err field. You are consuming the records without first checking for the Err.

khaf commented 4 years ago

I'm going to go ahead a close this issue. Feel free to file new ones.