tursodatabase / libsql-client-go

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

Client doesn't cleanly handle reconnection to different sqld instance; error "Received an invalid baton" #66

Closed CodingDoug closed 11 months ago

CodingDoug commented 11 months ago

With the following sequence of events:

  1. Start a sqld on localhost
  2. Client performs a simple query (select 1)
  3. Client sleeps
  4. Restart sqld (kill and run again)
  5. Client continues and performs another simple query (select 1)

The last query fails with "Received an invalid baton". After that, subsequent queries using the same client work again.

The client library should probably absorb these baton errors and retry internally (up to some limit?) so that the caller doesn't see them. The TypeScript SDK does not surface the same error in the same scenario.

This affects usage of the libsql shell as well as the Turso shell.

Might be related to #60, which shows the same error message.

bjornpagen commented 11 months ago

Seeing the same messages

honzasp commented 11 months ago

If you restart sqld, the SQL connection (Hrana stream) is gone; the "invalid baton" error is a symptom of this. It would be very incorrect if the client silently reopened the connection: imagine that the user executes statements BEGIN, DELETE FROM users, ROLLBACK; you definitely don't want to open a new connection between the BEGIN and the DELETE!

haaawk commented 11 months ago

This should reduce the amount of "Received an invalid baton" errors but they will still appear if the transaction is inactive for a long time. We can't totally remove it because of the reasons @honzasp listed.

CodingDoug commented 11 months ago

@haaawk @honzasp If we can't eliminate the problem, what can we do to communicate to the user of the SDK or CLI about what went wrong? The error message is not really helpful. They're not going to know what a baton is or what that means for their task at hand.

bjornpagen commented 11 months ago

I never use transactions, and alternatives like rqlite and Cloudflare D1 don't support them natively.

Just curious: could there be a mode to disable transactions to enable some sort of silent connection re-opening?

CodingDoug commented 11 months ago

The CLI doesn't do interactive transactions either, and would benefit from some internal reconnection logic.

haaawk commented 11 months ago

I never use transactions, and alternatives like rqlite and Cloudflare D1 don't support them natively.

Just curious: could there be a mode to disable transactions to enable some sort of silent connection re-opening?

In the latest version of the driver, connections should be reopened silently. Please update the driver and let me know if you're still seeing the baton errors @bjornpagen.

haaawk commented 11 months ago

@CodingDoug with the latest version of the driver no transactions => no baton errors any more

bjornpagen commented 11 months ago

looks fixed to me!

honzasp commented 11 months ago

However, what happens when the user does use transactions, or pragmas, or other features that require stateful connections? When the user executes statements BEGIN and DELETE FROM users, the driver must never ever reconnect between them.

haaawk commented 11 months ago

Here's how you do transactions @honzasp

tx, _ := db.BeginTx(ctx, nil)
defer tx.Rollback()
tx.ExecContext(ctx, "SQL")
tx.Commit()

That ensures the connection is kept alive for the duration of the transaction

honzasp commented 11 months ago

So if the user executes transaction commands manually, then it will be incorrect? And what about pragmas? Is this an expected fact of life of the Golang library?

haaawk commented 11 months ago

One can take a connection and use it for as long as they wish too. If not using transactions and not taking a connection explicitly, the user is expected to run self contained statements. This is my understanding of how the database/sql works.