trinodb / trino-go-client

Go client for Trino
Apache License 2.0
135 stars 63 forks source link

Can't execute more than one query on one connection #104

Closed sureshmula2 closed 9 months ago

sureshmula2 commented 9 months ago

I found an interesting thing, where I can't run two queries on a connection. I created a connection and set the query_max_execution_time to 10 minutes. Then, I execute only one query, I don't encounter any problem. However, if I execute more than one query, it fails with the following error. I believe this should not happen.


Error:          Received unexpected error:
                            <html>
                            <head>
                            <meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/>
                            <title>Error 500 Request failed.</title>
                            </head>
                            <body><h2>HTTP ERROR 500 Request failed.</h2>
                            <table>
                            <tr><th>URI:</th><td>/v1/statement</td></tr>
                            <tr><th>STATUS:</th><td>500</td></tr>
                            <tr><th>MESSAGE:</th><td>Request failed.</td></tr>
                            <tr><th>SERVLET:</th><td>org.glassfish.jersey.servlet.ServletContainer-6314b195</td></tr>
                            </table>

                            </body>
                            </html>

You can use following steps to reproduce the issue.

package main

import (
    "context"
    "fmt"

    "database/sql"
)

func check(ctx context.Context, config TrinoConfig) error {
    cfg, err := config.toTrinoDbConfig()
    dsn, err := cfg.FormatDSN()
    if err != nil {
        return err
    }
    db, err := sql.Open("trino", dsn)
    if err != nil {
        return err
    }

    timeoutStr := "10m"       // session timeout 10 minutes
    conn, err := db.Conn(ctx) //getting connection
    if err != nil {
        return err
    }
    _, err = conn.ExecContext(ctx, fmt.Sprintf("SET SESSION query_max_execution_time = '%s'", timeoutStr)) // setting session timeout as 10 minutes
    if err != nil {
        conn.Close()
        return err
    }

    _, err = conn.QueryContext(ctx, "select 1")
    if err != nil {
        return err
    }

    _, err = conn.QueryContext(ctx, "select 2")
    if err != nil {
        return err
    }
    return nil
}
sureshmula2 commented 9 months ago

@nineinchnick Anything on this?

nineinchnick commented 9 months ago

I can't reproduce this. What do you pass as the context?

sureshmula2 commented 9 months ago

The code is completely fine when I don't set the query_max_execution_time. There's an issue when setting the query_max_execution_time as it fails to execute two query statements.

func newTestContextHelper(t testing.TB, tzName string, testTimeout time.Duration) context.Context {
    // create a new context with the provided timeout duration.
    ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
    t.Cleanup(cancel)

    ctx = NewTimezoneContext(ctx, timezones.TzName(tzName))
    ctx = auth.NewContext(ctx, auth.NewClaims("XXX", "XX", "XX", "XX", "XX"))
    ctx = request.NewContext(
        ctx,
        request.NewRequestContext("XXX", "XX", "XX"))
    return ctx
}
nineinchnick commented 9 months ago

What's NewTimezoneContext?

I ran this program, connecting to Trino 437 running in a container:

package main

import (
    "context"
    "database/sql"
    "fmt"

    _ "github.com/trinodb/trino-go-client/trino"
)

func main() {
    check(context.Background())
}

func check(ctx context.Context) error {
    db, err := sql.Open("trino", "http://example@localhost:8080")
    if err != nil {
        return err
    }

    timeoutStr := "10m"       // session timeout 10 minutes
    conn, err := db.Conn(ctx) //getting connection
    if err != nil {
        return err
    }
    _, err = conn.ExecContext(ctx, fmt.Sprintf("SET SESSION query_max_execution_time = '%s'", timeoutStr)) // setting session timeout as 10 minutes
    if err != nil {
        conn.Close()
        return err
    }

    _, err = conn.QueryContext(ctx, "select 1")
    if err != nil {
        return err
    }

    _, err = conn.QueryContext(ctx, "select 2")
    if err != nil {
        return err
    }
    fmt.Printf("Fin!\n")
    return nil
}

It works just fine.

image

sureshmula2 commented 9 months ago

Apologies for pinging again, @nineinchnick.

Adding defer conn.Close() after creating a connection results in a panic. We're aiming to avoid hanging connections after executing queries. I also attempted to close the connection at the end of the program, but it still fails.

package main

import (
    "context"
    "database/sql"
    "fmt"

    _ "github.com/trinodb/trino-go-client/trino"
)

func main() {
    check(context.Background())
}

func check(ctx context.Context) error {
    db, err := sql.Open("trino", "http://example@localhost:8080")
    if err != nil {
        return err
    }

    timeoutStr := "10m"       // session timeout 10 minutes
    conn, err := db.Conn(ctx) //getting connection
        defer conn.Close()
    if err != nil {
        return err
    }
    _, err = conn.ExecContext(ctx, fmt.Sprintf("SET SESSION query_max_execution_time = '%s'", timeoutStr)) // setting session timeout as 10 minutes
    if err != nil {
        conn.Close()
        return err
    }

    _, err = conn.QueryContext(ctx, "select 1")
    if err != nil {
        return err
    }

    _, err = conn.QueryContext(ctx, "select 2")
    if err != nil {
        return err
    }
    fmt.Printf("Fin!\n")
    return nil
}
nineinchnick commented 9 months ago

When I ran it, it didn't panic, but got stuck. It works when I call close results of QueryContext(). This is expected, closing rows is required, if you don't need them you should use ExecContext() instead.

    rows, err = conn.QueryContext(ctx, "select 2")
    if err != nil {
        return err
    }
    err = rows.Close()
    if err != nil {
        return err
    }
sureshmula2 commented 9 months ago

Thank you. Leaving the connection open isn't ideal, and attempting to close it seems to cause it to get stuck. What course of action do you suggest in this situation?

nineinchnick commented 9 months ago

Trino connections are stateless, it's basically just a http connection. But if you close all other resources (statements, rows) you should be able to close the connection.

nineinchnick commented 9 months ago

Also the driver's close method is a noop, so it's not the driver's issue. This is how the Go's database/sql interface works.