tursodatabase / libsql-client-go

Go client API for libSQL
MIT License
159 stars 23 forks source link

Improve ResetSession by not sending requests #85

Closed haaawk closed 7 months ago

haaawk commented 7 months ago

Instead mark connection as closeStreamOnNextRequest and prepend StreamClose request to the pipeline before the next request.

Fixes #84

haaawk commented 7 months ago

I still have to test this since I'm not sure sqld will correctly handle closing the stream and opening a new on in a single request.

haaawk commented 7 months ago

Well. As great as this approach is it seems not to fly with sqld hrana implementation :(

error code 400: Stream for this baton was closed
haaawk commented 7 months ago

Tested with local sqld that includes https://github.com/tursodatabase/libsql/pull/621 and then it works.

penberg commented 7 months ago

@haaawk Since only the async close is enabled, i think we can merge this.

haaawk commented 7 months ago

@haaawk Since only the async close is enabled, i think we can merge this.

I think so.

haaawk commented 7 months ago

I found one more issue. With closing stream before next request not with async closing. Basically if a stream expires for example because the connection was resting in the pool for long enough. The request will fail. That would require some more fixes in sqld. Here's an easy reproducer:

db, err := sql.Open("libsql", dbUrl)
    if err != nil {
        fmt.Fprintf(os.Stderr, "failed to open db %s: %s", dbUrl, err)
        os.Exit(1)
    }
    for i := 0; i < 10; i++ {
        r, err := db.QueryContext(context.Background(), "SELECT 1")
        if err != nil {
            fmt.Fprintf(os.Stderr, "failed to query: %s", err)
            os.Exit(1)
        }
        err = r.Close()
        if err != nil {
            fmt.Fprintf(os.Stderr, "failed to close: %s", err)
            os.Exit(1)
        }
        time.Sleep(120 * time.Second)
    }

resulting in:

failed to query: failed to execute SQL: SELECT 1
error code 500: