Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: net: Add API to check connection Readability without blocking #28607

Closed
methane opened this issue Nov 6, 2018 · 6 comments

Comments

Projects
None yet
6 participants
@methane
Copy link
Contributor

commented Nov 6, 2018

I'm a maintainer of go-sql-driver/mysql.
Since MySQL protocol is a request-response protocol, we don't have dedicated goroutine for Write() and Read(). We call them in the goroutine executing DB.Query() .

Sometimes, DB connection is closed from a server or other middlewares.
But we can not detect connection closed by peer until we call Read().

Since we send a query before reading result, we can not retry the query. Otherwise, the query may be executed twice. See also: https://golang.org/pkg/database/sql/driver/

To prevent duplicate operations, ErrBadConn should NOT be returned if there's a possibility that the database server might have performed the operation. Even if the server sends back an error, you shouldn't return ErrBadConn.

If we can detect connection closed by peer before sending a query, we can retry safely. But there is no easy way to do it now.

  • Dedicated reader goroutine increase complexity. It will increase memory usage because we can not use same buffer for Read() and Write(). It will increase allocation because received data should be passed to the goroutine executing DB.Query().
  • conn.SetReadDeadline(time.Now().Add(time.Millisecond)); conn.Read() introduces unnecessary sleep for normal case.
  • Using SyscallConn is not easy, especially for cross platform.

So I want a easy way to check EOF. I have two ideas about it:

  • c.Readable() bool -- Returns true when c.Read() will not block.
  • c.ReadNonblock(buff []bytes) (int, error) -- Nonblocking variants of c.Read().

@gopherbot gopherbot added this to the Proposal milestone Nov 6, 2018

@gopherbot gopherbot added the Proposal label Nov 6, 2018

@methane methane changed the title Proposal: Add API for check EOF without block Proposal: net: Add API to check EOF without block Nov 6, 2018

@agnivade

This comment has been minimized.

Copy link
Member

commented Nov 6, 2018

Is the (*bufio).Peek method sufficient ? How do other sql drivers handle this scenario ?

/cc @kardianos

@kardianos

This comment has been minimized.

Copy link
Contributor

commented Nov 6, 2018

The MS SQL driver spins up goroutines. That's what I recommen MySQL doing too.

Your proposal ends up racy or spinning I believe if you tried it. There was one proposal to expose the polling interface from the runtime, but I don't think it went anywhere.

@kardianos kardianos closed this Nov 6, 2018

@kardianos kardianos reopened this Nov 6, 2018

@methane

This comment has been minimized.

Copy link
Contributor Author

commented Nov 6, 2018

@kardianos Would you point out where go-mssqldb uses goroutine to check connection closed while it is idle (not waiting response)?

@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2018

I'm confused about the proposal. The title says "check EOF" but the text seems to be about "check whether Read will proceed without blocking" (data available or EOF).

Assuming this is really about readability and not specifically EOF, this is a duplicate of #15735.

(Operating systems do provide us ways to watch general readability. I'm not sure how to watch EOF (without reading).) Closing as duplicate of #15735, since that's the one we could actually implement.

@rsc rsc closed this Nov 28, 2018

@methane

This comment has been minimized.

Copy link
Contributor Author

commented Nov 29, 2018

@rsc It bit different. I want to check readable without blocking. I don't want to wait until readable. I don't want callbacks.
This is pseudo code:

func (mc mysqlConn) Query(query string, args... driver.Value) error {
    if mc.conn.Readable() {
        // Connection is closed from server.
        return mc.handleServerGone()
    }

    mc.sendQuery(query, args...)
    return mc.recvResult()
}

@methane methane changed the title Proposal: net: Add API to check EOF without block Proposal: net: Add API to check connection Readability without blocking Nov 29, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Nov 29, 2018

@methane I suggest that you follow up on #15735. It sounds very similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.