ccl/sqlproxyccl: replace ConnectionCopy logic in forwarder with interceptors

[jaylim-crl]

pgwire: expose NewReadTimeoutConn to be used outside of the pgwire package

Previously, readTimeoutConn was only used within the pgwire package to read
messages from the client for the SQL server. We want this construct when
reading messages from clients within the sqlproxy as well. This commit exposes
the readTimeoutConn construct through NewReadTimeoutConn.

ccl/sqlproxyccl: replace ConnectionCopy logic in forwarder with interceptors

Previously, we were using io.Copy through ConnectionCopy to forward messages
between the client and SQL server. Now that the interceptors have been merged,
we will update the forwarder to use these interceptors instead of the old
approach.

There are a few notable changes in this commit:

1. We wrap clientConn with the a readTimeoutConn that was exposed in the
   previous commit. This allows us to unblock on Read whenever an activity
   occurs (e.g. context cancellation, and in the future, when transfer is
   requested).
2. There are two goroutines per connection within the forwarder: one for the
   request processor (client to server), and the other for the response
   processor (server to client).
3. We also removed unnecessary checks for codeExpiredClientConnection and
   codeIdleDisconnect errors when copying from server to client. These are
   already handled by the idle monitor's callback as well as the denylist
   watcher's callback. When those constructs were implemented back then, we
   did not remove them from ConnectionCopy.

We need to clean up error messages, and how we propagate them back to the
user because today we just close the connection without returning a response,
resulting in, I believe, a broken pipe error.

Release note: None

[cockroach-teamcity]

This change is 

[jaylim-crl]

I'm putting this PR up for review first. Tests will come later once we're aligned on the design. Existing sqlproxyccl tests are all passing - we'll just need to add new ones. As a reminder, only the last two commits are relevant. The first has been reviewed.

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/ccl/sqlproxyccl/forwarder.go, line 81 at r2 (raw file):

		// once the context gets cancelled. This is fine for now since we'll
		// be removing this soon anyway.
		f.Close()

This is removed because we no longer need this :)

Code quote:

		// Close the forwarder to clean up. This goroutine is temporarily here
		// because the only way to unblock io.Copy is to close one of the ends,
		// which will be done through closing the forwarder. Once we replace
		// io.Copy with the interceptors, we could use f.ctx directly, and no
		// longer need this goroutine.
		//
		// Note that if f.Close was called externally, this will result
		// in two f.Close calls in total, i.e. one externally, and one here
		// once the context gets cancelled. This is fine for now since we'll
		// be removing this soon anyway.
		f.Close()

---

pkg/ccl/sqlproxyccl/forwarder.go, line 120 at r3 (raw file):

	if err != nil {
		// This will not happen because defaultBufferSize > 5.
		panic(errors.NewAssertionErrorWithWrappedErrf(err, "client interceptor"))

I'd rather we panic here instead of handling the extra error that we won't need in the caller.

---

pkg/ccl/sqlproxyccl/forwarder.go, line 204 at r3 (raw file):

}

// wrapServerToClientError overrides client to server errors for external

Typo here, but I'll fix it later. Should be "server to client" instead.

[jaylim-crl]

This PR is ready for a review. Only review the last two commits. The others are currently in the merge queue; they have been in there for hours and bors is unhappy. Once they are in master, I'll rebase.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball and @jeffswenson)

---

pkg/ccl/sqlproxyccl/forwarder.go, line 120 at r3 (raw file):

Previously, jaylim-crl (Jay Lim) wrote…

I'd rather we panic here instead of handling the extra error that we won't need in the caller.

No longer needed.

---

pkg/ccl/sqlproxyccl/forwarder.go, line 204 at r3 (raw file):

Previously, jaylim-crl (Jay Lim) wrote…

Typo here, but I'll fix it later. Should be "server to client" instead.

Done.

[jeffswenson]

LGTM

[jaylim-crl]

TFTR!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball)

---

pkg/ccl/sqlproxyccl/forwarder.go, line 154 at r5 (raw file):

Previously, JeffSwenson (Jeff Swenson) wrote…

nit: wrapClientToServerError and wrapServerToClientError can be implemented as functions instead of methods.

Done.

[jaylim-crl]

bors r=JeffSwenson

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build succeeded:

• GitHub CI (Cockroach)