cockroachdb / cockroach-go

Packages for go clients.

CockroachDB Go Helpers

This project contains helpers for CockroachDB users writing in Go:

• crdb and its subpackages provide wrapper functions for retrying transactions that fail due to serialization errors. It is intended for use within any Go application. See crdb/README.md for more details.
• testserver provides functions for starting and connecting to a locally running instance of CockroachDB. It is intended for use in test code.

Prerequisites

The current release (v2) of this library requires Go modules.

You can import it in your code using:

import (
	"github.com/cockroachdb/cockroach-go/v2/crdb"
	"github.com/cockroachdb/cockroach-go/v2/testserver"
)

cockroach-go's Issues

fix glibc/musl detection

We should revert #31 once jemalloc/jemalloc#585 is fixed. We may also want to expand the glibc regex to match "EGLIBC" and not just "GLIBC." The internet suggests ldd --version may report "GNU libc" on some distros.

binary download should use s3 website endpoint and follow the redirect

We currently issue two web requests, first fetching .LATEST, then reading its content, and finally downloading the binary itself. Instead, we should hit the "website" endpoint, e.g. http://cockroach.s3-website-us-east-1.amazonaws.com/cockroach/cockroach.darwin-amd64.LATEST, and just follow the redirect.

We should also make that link work over HTTPS, which it currently does not.

Assigning @mberhault for the SSL bit.

Add support for jackc/pgx/v5

jackc/pgx introduced a new major version v5

testserver: cmd with `--log` option does not generate `listen-url` in tests

After replacing --logtostderr to --log="{file-defaults: {dir: tmp/dir/to/log}, sinks: {stderr: {filter: NONE}}}" , the "cockroach" command does not generate the listen-url file in the tmp directory when running tests from testserver_test.go. But the same command works well in local CLI.

Demo of tests with original --logtostderr running in debugger: check here
Demo of tests with --log running in debugger and local CLI: check here

Demos are based on these code changes.

Second call to Start() throws error

The current documentation instructs that the method Start should be called after creating a new test server. However, calling this method throws the error: Start() can only be called once

https://github.com/cockroachdb/cockroach-go/blob/master/testserver/testserver.go#L30

ts, err := testserver.NewTestServer()
if err != nil {
    t.Fatal(err)
}

if err := ts.Start(); err != nil { 
    t.Fatal(err) <== "Start() can only be called once"
}

defer ts.Stop()

The first call to the method is happening in https://github.com/cockroachdb/cockroach-go/blob/master/testserver/testserver.go#L328.

Expected:
Remove the following block from he documentation.

if err := ts.Start(); err != nil { 
    t.Fatal(err)
}

Need variants of ExecuteInTx and ExecuteTx that take a `fn(context.Context, ...)` as arg

The ExecuteTx and ExecuteInTx APIs take a function callback that only takes either no argument or a tx object.

We want to pass a callback instead that takes the context as argument.

cc @rafiss

testserver: if killed during download, future invocations will not work

Our checking doesn't use a flock file or anything like that. Instead it assumes that if the file exists but does not have the finished perms that some other process must be downloading it.

Sync changes from master to version-module branch

Recently added sqlx.Tx adapter support to crdb.Tx, ref: 0b2acdb - What is the best way to get this change ported over to version-module branch and cut a release (say v2.1.0 or something)?

cc @andreimatei @rafiss

testserver: automatically download only stable cockroachdb versions

Right now testserver downloads any latest builds of Cockroachdb, this includes alphas, betas, etc.
I think it should download only stable versions by default instead. Or at least add an option for it.

This was already discussed in #19. But it seems that there was no resolution and the thread isn't active anymore.

security: Use `/var/tmp` safely

In a multi-user system, the testserver binary could have been written by another user and may be malicious. We should include the username in the path (either in the filename or as an intermediate directory; I think a directory might be a little safer) and verify that ownership and permissions are as expected.

testserver: Failed to fetch latest binary - file already exists

Check out this test failure:
https://teamcity.cockroachdb.com/viewLog.html?buildId=285400&buildTypeId=Cockroach_CockroachGo

And the same thing happened on my machine too earlier too and went away when I deleted the file that it complains about already existing.

testserver.go:128: failure starting process: exec: "cockroach": executable file not found in $PATH
------- Stdout: -------
2017/07/01 04:32:55 GET https://edge-binaries.cockroachdb.com/cockroach/cockroach.linux-gnu-amd64.LATEST
2017/07/01 04:32:55 Failed to fetch latest binary: error creating /home/agent/temp/buildTmp/cockroach.linux-gnu-amd64.837b8d054d5190ea6c11adf54e3bf3f25cd63938: open /home/agent/temp/buildTmp/cockroach.linux-gnu-amd64.837b8d054d5190ea6c11adf54e3bf3f25cd63938: file exists, attempting to use cockroach binary from your PATH
2017/07/01 04:32:55 exec: "cockroach": executable file not found in $PATH

TestRunServer/SecureTenant is broken with CockroachDB master

This test is currently broken:

=== RUN   TestRunServer
=== RUN   TestRunServer/SecureTenant
2020/08/06 16:17:51 Using custom cockroach binary: /Users/rafiss/go/src/github.com/cockroachdb/cockroach/cockroach
2020/08/06 16:17:51 process 95859 started: /Users/rafiss/go/src/github.com/cockroachdb/cockroach/cockroach start-single-node --logtostderr --certs-dir=/tmp/cockroach-testserver274475142/certs --host=localhost --port=0 --http-port=0 --store=/tmp/cockroach-testserver274475142 --listening-url-file=/tmp/cockroach-testserver274475142/listen-url
2020/08/06 16:17:53 process 95886 started: /Users/rafiss/go/src/github.com/cockroachdb/cockroach/cockroach mt start-sql --certs-dir=/tmp/cockroach-testserver274475142/certs --logtostderr --tenant-id=2 --kv-addrs=localhost:50535 --sql-addr=localhost:50545
2020/08/06 16:17:53 WaitForInit: Trying again after error: dial tcp [::1]:50545: connect: connection refused
2020/08/06 16:17:53 Process 95886 exited with status 2: exit status 2
2020/08/06 16:17:53 exit status 2
2020/08/06 16:17:53 WaitForInit: Trying again after error: dial tcp [::1]:50545: connect: connection refused
2020/08/06 16:17:53 WaitForInit: Trying again after error: dial tcp [::1]:50545: connect: connection refused
2020/08/06 16:17:53 WaitForInit: Trying again after error: dial tcp [::1]:50545: connect: connection refused
2020/08/06 16:17:53 WaitForInit: Trying again after error: dial tcp [::1]:50545: connect: connection refused
...

--- FAIL: TestRunServer (7.58s)
    testserver_test.go:44: dial tcp [::1]:50545: connect: connection refused
    --- FAIL: TestRunServer/SecureTenant (7.58s)

I was able to reproduce locally, and see this error when starting the tenant:

❯ /Users/rafiss/go/src/github.com/cockroachdb/cockroach/cockroach mt start-sql --certs-dir=/tmp/cockroach-testserver274475142/certs --logtostderr --tenant-id=2 --kv-addrs=localhost:50535 --sql-addr=localhost:50545
I200806 20:18:51.231010 15 util/stop/stopper.go:563  quiescing
E200806 20:18:51.230998 1 util/log/crash_reporting.go:138  a panic has occurred!
for 2: tenant client cert not found
(1) attached stack trace
  | runtime.gopanic
  | 	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/panic.go:967
  | github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).Stop
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:503
  | runtime.gopanic
  | 	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/panic.go:967
  | github.com/cockroachdb/cockroach/pkg/rpc.NewServer
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/rpc/context.go:216
  | github.com/cockroachdb/cockroach/pkg/server.makeSQLServerArgs
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/server/testserver.go:561
  | github.com/cockroachdb/cockroach/pkg/server.StartTenant
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/server/testserver.go:641
  | github.com/cockroachdb/cockroach/pkg/cli.runStartSQL
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/mt_start_sql.go:68
  | github.com/cockroachdb/cockroach/pkg/cli.MaybeDecorateGRPCError.func1
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/error.go:189
  | github.com/spf13/cobra.(*Command).execute
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/vendor/github.com/spf13/cobra/command.go:826
  | github.com/spf13/cobra.(*Command).ExecuteC
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/vendor/github.com/spf13/cobra/command.go:914
  | github.com/spf13/cobra.(*Command).Execute
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/vendor/github.com/spf13/cobra/command.go:864
  | github.com/cockroachdb/cockroach/pkg/cli.Run
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/cli.go:225
  | github.com/cockroachdb/cockroach/pkg/cli.Main
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/cli.go:67
  | main.main
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cmd/cockroach-short/main.go:26
  | runtime.main
  | 	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/proc.go:203
  | runtime.goexit
  | 	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/asm_amd64.s:1373
Wraps: (2) for 2: tenant client cert not found
Error types: (1) *withstack.withStack (2) *security.Error
E200806 20:18:51.231616 1 util/log/crash_reporting.go:138  a panic has occurred!
for 2: tenant client cert not found
(1) attached stack trace
  | runtime.gopanic
  | 	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/panic.go:967
  | github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).Stop
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:503
  | runtime.gopanic
  | 	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/panic.go:967
  | github.com/cockroachdb/cockroach/pkg/rpc.NewServer
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/rpc/context.go:216
  | github.com/cockroachdb/cockroach/pkg/server.makeSQLServerArgs
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/server/testserver.go:561
  | github.com/cockroachdb/cockroach/pkg/server.StartTenant
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/server/testserver.go:641
  | github.com/cockroachdb/cockroach/pkg/cli.runStartSQL
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/mt_start_sql.go:68
  | github.com/cockroachdb/cockroach/pkg/cli.MaybeDecorateGRPCError.func1
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/error.go:189
  | github.com/spf13/cobra.(*Command).execute
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/vendor/github.com/spf13/cobra/command.go:826
  | github.com/spf13/cobra.(*Command).ExecuteC
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/vendor/github.com/spf13/cobra/command.go:914
  | github.com/spf13/cobra.(*Command).Execute
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/vendor/github.com/spf13/cobra/command.go:864
  | github.com/cockroachdb/cockroach/pkg/cli.Run
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/cli.go:225
  | github.com/cockroachdb/cockroach/pkg/cli.Main
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/cli.go:67
  | main.main
  | 	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cmd/cockroach-short/main.go:26
  | runtime.main
  | 	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/proc.go:203
  | runtime.goexit
  | 	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/asm_amd64.s:1373
Wraps: (2) for 2: tenant client cert not found
Error types: (1) *withstack.withStack (2) *security.Error
panic: for 2: tenant client cert not found [recovered]
	panic: for 2: tenant client cert not found [recovered]
	panic: for 2: tenant client cert not found [recovered]
	panic: for 2: tenant client cert not found

goroutine 1 [running]:
github.com/cockroachdb/cockroach/pkg/util/log.RecoverAndReportPanic(0x9091a60, 0xc00007a160, 0xc000016a80)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/util/log/crash_reporting.go:88 +0x9f
panic(0x7e15f20, 0xc000da45c0)
	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/panic.go:967 +0x166
github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).Recover(0xc00011fdd0, 0x9091a60, 0xc00007a160)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:207 +0x11f
panic(0x7e15f20, 0xc000da45c0)
	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/panic.go:967 +0x166
github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).Stop(0xc00011fdd0, 0x9091a60, 0xc00007a160)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:503 +0x68c
panic(0x7e15f20, 0xc000da45c0)
	/usr/local/Cellar/go/1.14.1/libexec/src/runtime/panic.go:967 +0x166
github.com/cockroachdb/cockroach/pkg/rpc.NewServer(0xc0000f6240, 0x0, 0x0, 0x0, 0x90db8a0)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/rpc/context.go:216 +0xe7c
github.com/cockroachdb/cockroach/pkg/server.makeSQLServerArgs(0xc00011fdd0, 0x0, 0x0, 0xc000016a80, 0xc000197400, 0x906e860, 0xc000adf8a0, 0x0, 0x0, 0x0, ...)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/server/testserver.go:561 +0xbda
github.com/cockroachdb/cockroach/pkg/server.StartTenant(0x9091a60, 0xc00007a160, 0xc00011fdd0, 0x0, 0x0, 0xc000016a80, 0xc000197400, 0x906e860, 0xc000adf8a0, 0x0, ...)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/server/testserver.go:641 +0xf8
github.com/cockroachdb/cockroach/pkg/cli.runStartSQL(0xb7685c0, 0xc000b0b450, 0x0, 0x5, 0x0, 0x0)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/mt_start_sql.go:68 +0x2c4
github.com/cockroachdb/cockroach/pkg/cli.MaybeDecorateGRPCError.func1(0xb7685c0, 0xc000b0b450, 0x0, 0x5, 0x0, 0x0)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/error.go:189 +0x79
github.com/spf13/cobra.(*Command).execute(0xb7685c0, 0xc000b0b400, 0x5, 0x5, 0xb7685c0, 0xc000b0b400)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/vendor/github.com/spf13/cobra/command.go:826 +0x453
github.com/spf13/cobra.(*Command).ExecuteC(0xb75f740, 0xc00007a160, 0xc00062b62a, 0xc)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/vendor/github.com/spf13/cobra/command.go:914 +0x2fb
github.com/spf13/cobra.(*Command).Execute(...)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/vendor/github.com/spf13/cobra/command.go:864
github.com/cockroachdb/cockroach/pkg/cli.Run(...)
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/cli.go:225
github.com/cockroachdb/cockroach/pkg/cli.Main()
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cli/cli.go:67 +0x270
main.main()
	/Users/rafiss/go/src/github.com/cockroachdb/cockroach/pkg/cmd/cockroach-short/main.go:26 +0x20

testserver: broken with latest build

ts.WaitForInit(db) blocks indefinitely on my system (Ubuntu 16.04). Is it possible that the latest cockroachdb build breaks the testserver package?

I added a manual break after 5 seconds of iterating and logged the error and stdout/stderr from the testserver;

func (ts *TestServer) WaitForInit(db *sql.DB) {
	cur, max := 0, 500
	for {
		// We issue a query that fails both on connection errors and on the
		// system database not existing.
		if _, err := db.Query("SHOW DATABASES"); err == nil {
			return
		} else if cur > max {
			log.Println(ts.Stdout())
			log.Println(ts.Stderr())
			panic(err)
		}
		cur++
		time.Sleep(time.Millisecond * 10)
	}
}

Output (cleaned up):

2016/12/04 15:52:31 downloading https://s3.amazonaws.com/cockroach/cockroach/cockroach.linux-amd64.9111f757330103a89ccc9ef735d9c37867d71706 to /var/tmp/cockroach.linux-amd64.9111f757330103a89ccc9ef735d9c37867d71706, this may take some time
2016/12/04 15:52:44 Using automatically-downloaded binary: /var/tmp/cockroach.linux-amd64.9111f757330103a89ccc9ef735d9c37867d71706
2016/12/04 15:52:44 process 15520 started: /var/tmp/cockroach.linux-amd64.9111f757330103a89ccc9ef735d9c37867d71706 start --logtostderr --insecure --port=0 --http-port=0 --socket=/tmp/cockroach-testserver200397412/.s.PGSQL.26257 --store=/tmp/cockroach-testserver200397412
waiting for database to initialize
2016/12/04 15:52:50 CockroachDB node starting at 2016-12-04 15:52:44.767370201 +0100 CET

--- FAIL: TestRunServer (19.29s)
panic: pq: database "information_schema" does not exist [recovered]
    panic: pq: database "information_schema" does not exist

Testserver Stdout:

build:      9111f75 @ 2016/12/04 13:07:50 (go1.7.3)
admin:      http://m47:45529
sql:        postgresql://root@m47:40251?sslmode=disable
socket:     /tmp/cockroach-testserver200397412/.s.PGSQL.26257
logs:       /tmp/cockroach-testserver200397412/logs
store[0]:   path=/tmp/cockroach-testserver200397412
status:     initialized new cluster
clusterID:  fcbd6d02-c65b-4930-be7f-6859769ab779
nodeID:     1

Testserver Stderr:

2016/12/04 15:52:50 I161204 15:52:44.622880 1 cli/start.go:318  CockroachDB 9111f75 (linux amd64, built 2016/12/04 13:07:50, go1.7.3)
I161204 15:52:44.723164 1 cli/start.go:334  starting cockroach node
I161204 15:52:44.723274 1 cli/start.go:336  using local environment variables: COCKROACH_MAX_OFFSET
W161204 15:52:44.723330 1 server/server.go:154  [n?] running in insecure mode, this is strongly discouraged. See --insecure.
I161204 15:52:44.723455 1 gossip/gossip.go:248  [n?] initial resolvers: []
W161204 15:52:44.723495 1 gossip/gossip.go:1124  [n?] no resolvers found; use --join to specify a connected node
I161204 15:52:44.725427 1 storage/engine/rocksdb.go:326  opening rocksdb instance at "/tmp/cockroach-testserver200397412"
I161204 15:52:44.730046 50 kv/dist_sender.go:302  [n?] unable to determine this node's attributes for replica selection; node is most likely bootstrapping
I161204 15:52:44.759296 1 server/config.go:443  1 storage engine initialized
I161204 15:52:44.759864 1 server/node.go:419  [n?] store [n0,s0] not bootstrapped
I161204 15:52:44.761700 63 storage/replica_proposal.go:348  [s1,r1/1:/M{in-ax},@c420276900] new range lease replica {1 1 1} 1970-01-01 00:00:00 +0000 UTC 411350h52m53.761170701s following replica {0 0 0} 1970-01-01 00:00:00 +0000 UTC 0s [physicalTime=2016-12-04 14:52:44.761649824 +0000 UTC]
I161204 15:52:44.762300 1 util/stop/stopper.go:396  stop has been called, stopping or quiescing all running tasks
I161204 15:52:44.762384 1 server/node.go:348  [n?] **** cluster fcbd6d02-c65b-4930-be7f-6859769ab779 has been created
I161204 15:52:44.762404 1 server/node.go:349  [n?] **** add additional nodes by specifying --join=m47:40251
I161204 15:52:44.762671 1 base/node_id.go:62  [n1] NodeID set to 1
I161204 15:52:44.763014 1 storage/store.go:1223  [n1] [n1,s1]: failed initial metrics computation: [n1,s1]: system config not yet available
I161204 15:52:44.763051 1 server/node.go:432  [n1] initialized store [n1,s1]: {Capacity:105555197952 Available:61341224960 RangeCount:1 LeaseCount:1}
I161204 15:52:44.763078 1 server/node.go:317  [n1] node ID 1 initialized
I161204 15:52:44.763156 1 gossip/gossip.go:290  [n1] NodeDescriptor set to node_id:1 address:<network_field:"tcp" address_field:"m47:40251" > attrs:<> locality:<> 
I161204 15:52:44.763341 1 storage/stores.go:296  [n1] read 0 node addresses from persistent storage
I161204 15:52:44.763500 1 server/node.go:562  [n1] connecting to gossip network to verify cluster ID...
I161204 15:52:44.763535 1 server/node.go:582  [n1] node connected via gossip and verified as part of cluster "fcbd6d02-c65b-4930-be7f-6859769ab779"
I161204 15:52:44.763582 1 server/node.go:367  [n1] node=1: started with [[]=/tmp/cockroach-testserver200397412] engine(s) and attributes []
I161204 15:52:44.763635 1 sql/executor.go:291  [n1] creating distSQLPlanner with address {tcp m47:40251}
I161204 15:52:44.765237 28 storage/split_queue.go:103  [n1,split,s1,r1/1:/M{in-ax},@c420277200] splitting at keys [/Table/11/0 /Table/12/0 /Table/13/0 /Table/14/0]
I161204 15:52:44.766902 28 storage/replica_command.go:2369  [n1,split,s1,r1/1:/M{in-ax},@c420277200] initiating a split of this range at key /Table/11 [r2]
I161204 15:52:44.767156 1 server/server.go:633  [n1] starting http server at m47:45529
I161204 15:52:44.767177 1 server/server.go:634  [n1] starting grpc/postgres server at m47:40251
I161204 15:52:44.767198 1 server/server.go:635  [n1] advertising CockroachDB node at m47:40251
I161204 15:52:44.767261 1 server/server.go:637  [n1] starting postgres server at unix:/tmp/cockroach-testserver200397412/.s.PGSQL.26257
I161204 15:52:44.768282 106 server/updates.go:158  [n1,updates check] No previous updates check time.
E161204 15:52:44.775774 29 storage/queue.go:609  [n1,replicate,s1,r1/1:/{Min-Table/11},@c420277200] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.775884 28 storage/queue.go:598  [n1,split,s1,r1/1:/{Min-Table/11},@c420277200] unable to split [n1,s1,r1/1:/{Min-Table/11}] at key "/Table/12/0": key range /Table/12/0-/Table/12/0 outside of bounds of range /Min-/Max
E161204 15:52:44.775933 110 storage/queue.go:609  [n1,replicate,s1,r1/1:/{Min-Table/11},@c420277200] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.776003 110 storage/queue.go:609  [n1,replicate,s1,r1/1:/{Min-Table/11},@c420277200] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
I161204 15:52:44.776052 28 storage/split_queue.go:103  [n1,split,s1,r2/1:/{Table/11-Max},@c420262480] splitting at keys [/Table/12/0 /Table/13/0 /Table/14/0]
I161204 15:52:44.776098 28 storage/replica_command.go:2369  [n1,split,s1,r2/1:/{Table/11-Max},@c420262480] initiating a split of this range at key /Table/12 [r3]
E161204 15:52:44.780195 29 storage/queue.go:609  [n1,replicate,s1,r2/1:/Table/1{1-2},@c420262480] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.780334 28 storage/queue.go:598  [n1,split,s1,r2/1:/Table/1{1-2},@c420262480] unable to split [n1,s1,r2/1:/Table/1{1-2}] at key "/Table/13/0": key range /Table/13/0-/Table/13/0 outside of bounds of range /Table/11-/Max
E161204 15:52:44.780492 110 storage/queue.go:609  [n1,replicate,s1,r1/1:/{Min-Table/11},@c420277200] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.780576 110 storage/queue.go:609  [n1,replicate,s1,r2/1:/Table/1{1-2},@c420262480] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
I161204 15:52:44.780705 28 storage/split_queue.go:103  [n1,split,s1,r3/1:/{Table/12-Max},@c420262d80] splitting at keys [/Table/13/0 /Table/14/0]
I161204 15:52:44.780758 28 storage/replica_command.go:2369  [n1,split,s1,r3/1:/{Table/12-Max},@c420262d80] initiating a split of this range at key /Table/13 [r4]
E161204 15:52:44.784909 29 storage/queue.go:609  [n1,replicate,s1,r3/1:/Table/1{2-3},@c420262d80] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.784938 28 storage/queue.go:598  [n1,split,s1,r3/1:/Table/1{2-3},@c420262d80] unable to split [n1,s1,r3/1:/Table/1{2-3}] at key "/Table/14/0": key range /Table/14/0-/Table/14/0 outside of bounds of range /Table/12-/Max
I161204 15:52:44.785024 28 storage/split_queue.go:103  [n1,split,s1,r4/1:/{Table/13-Max},@c4201ea900] splitting at keys [/Table/14/0]
I161204 15:52:44.785064 28 storage/replica_command.go:2369  [n1,split,s1,r4/1:/{Table/13-Max},@c4201ea900] initiating a split of this range at key /Table/14 [r5]
E161204 15:52:44.785120 110 storage/queue.go:609  [n1,replicate,s1,r1/1:/{Min-Table/11},@c420277200] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.785190 110 storage/queue.go:609  [n1,replicate,s1,r2/1:/Table/1{1-2},@c420262480] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.785246 110 storage/queue.go:609  [n1,replicate,s1,r3/1:/Table/1{2-3},@c420262d80] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.789105 29 storage/queue.go:609  [n1,replicate,s1,r4/1:/Table/1{3-4},@c4201ea900] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.789255 29 storage/queue.go:609  [n1,replicate,s1,r5/1:/{Table/14-Max},@c42024b680] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.789312 110 storage/queue.go:609  [n1,replicate,s1,r1/1:/{Min-Table/11},@c420277200] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.789372 110 storage/queue.go:609  [n1,replicate,s1,r2/1:/Table/1{1-2},@c420262480] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.789426 110 storage/queue.go:609  [n1,replicate,s1,r3/1:/Table/1{2-3},@c420262d80] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
E161204 15:52:44.789477 110 storage/queue.go:609  [n1,replicate,s1,r4/1:/Table/1{3-4},@c4201ea900] purgatory: 0 of 1 store with an attribute matching []; likely not enough nodes in cluster
I161204 15:52:44.811683 134 sql/event_log.go:95  [n1] Event: "node_join", target: 1, info: {Descriptor:{NodeID:1 Address:{NetworkField:tcp AddressField:m47:40251} Attrs: Locality:} ClusterID:fcbd6d02-c65b-4930-be7f-6859769ab779 StartedAt:1480863164763543657}
I161204 15:52:45.353809 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160428, details: https://www.cockroachlabs.com/docs/beta-20160428.html
I161204 15:52:45.353825 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160505, details: https://www.cockroachlabs.com/docs/beta-20160505.html
I161204 15:52:45.353833 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160512, details: https://www.cockroachlabs.com/docs/beta-20160512.html
I161204 15:52:45.353843 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160519, details: https://www.cockroachlabs.com/docs/beta-20160519.html
I161204 15:52:45.353851 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160526, details: https://www.cockroachlabs.com/docs/beta-20160526.html
I161204 15:52:45.353860 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160602, details: https://www.cockroachlabs.com/docs/beta-20160602.html
I161204 15:52:45.353868 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160609, details: https://www.cockroachlabs.com/docs/beta-20160609.html
I161204 15:52:45.353876 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160616, details: https://www.cockroachlabs.com/docs/beta-20160616.html
I161204 15:52:45.353885 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160714, details: https://www.cockroachlabs.com/docs/beta-20160714.html
I161204 15:52:45.353893 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160728, details: https://www.cockroachlabs.com/docs/beta-20160728.html
I161204 15:52:45.353901 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160908, details: https://www.cockroachlabs.com/docs/beta-20160908.html
I161204 15:52:45.353909 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160915, details: https://www.cockroachlabs.com/docs/beta-20160915.html
I161204 15:52:45.353921 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20160929, details: https://www.cockroachlabs.com/docs/beta-20160929.html
I161204 15:52:45.353930 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20161006, details: https://www.cockroachlabs.com/docs/beta-20161006.html
I161204 15:52:45.353938 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20161013, details: https://www.cockroachlabs.com/docs/beta-20161013.html
I161204 15:52:45.353946 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20161027, details: https://www.cockroachlabs.com/docs/beta-20161027.html
I161204 15:52:45.353956 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20161103, details: https://www.cockroachlabs.com/docs/beta-20161103.html
I161204 15:52:45.353964 106 server/updates.go:213  [n1,updates check] A new version is available: beta-20161201, details: https://www.cockroachlabs.com/docs/beta-20161201.html

should use `errors.Cause(err)` instead of `err` to check for retryable error

to allow this code to work correctly:

if err := tx.Exec("...."); err != nil {
    return errors.Wrap(err, "during insert")
}

Should be 25P02 retriable?

We are having problems in our tests, mainly are 40001 errors.

I integrated the fantastic cockroach-go retry mechanism by wrapping pgx here:

https://github.com/lopezator/crdbx

The retry mechanism here helps, but I wasn't able to pass all tests due to some weird 25P02 errors.

Should this error code be retriable?

user_test.go:1163: request failed: crdb: failed to execute a one query for users: bind failed to execute query: ERROR: current transaction is aborted, commands ignored until end of transaction block (SQLSTATE 25P02)

I suspected this was due to some t.Parallel issuing UPDATE stmts over the same set of data simultaneously, so I just changed the seed data for the conflicting tests and worked, but we obviously cannot rely on this.

Maybe is just something that has to be fixed on cockroachdb and not here, but we could retry those errors here in the meantime...

Thoughts?

crdbpgx: compilation error with go modules

I setup a simple package to try crdbpgx with the latest pgx/v4. I used go modules to manage my dependencies. I get the following error when trying to compile my package:

# github.com/cockroachdb/cockroach-go/crdb/crdbpgx
../../go/pkg/mod/github.com/cockroachdb/[email protected]/crdb/crdbpgx/pgx.go:54:21: too many arguments in call to tx.tx.Commit
	have (context.Context)
	want ()
../../go/pkg/mod/github.com/cockroachdb/[email protected]/crdb/crdbpgx/pgx.go:58:23: too many arguments in call to tx.tx.Rollback
	have (context.Context)
	want ()
../../go/pkg/mod/github.com/cockroachdb/[email protected]/crdb/crdbpgx/pgx.go:63:22: too many arguments in call to tx.tx.Exec
	have (context.Context, string, []interface {}...)
	want (string, ...interface {})

Source code of my package: here
The problem occurs because go mod also install the previous version of pgx - v3, that is referenced somehow internally. Corresponding part in my go.sum file:

github.com/jackc/pgx v3.6.2+incompatible h1:2zP5OD7kiyR3xzRYMhOcXVvkDZsImVXfj+yIyTQf3/o=
github.com/jackc/pgx v3.6.2+incompatible/go.mod h1:0ZGrqGqkRlliWnWB4zKnWtjbSWbGkVEFm4TeybAXq+I=

I understand that the original problem is with pgx library and when they fix their dependencies the problem will disappear. But maybe it would be beneficial to start using go modules in crdb package to be protected in such cases?

original error clobbered in retry loop if ROLLBACK TO SAVEPOINT also fails

This line should combine the errors:

cockroach-go/crdb/tx.go

Line 106 in 41e9cea

if _, err = tx.Exec("ROLLBACK TO SAVEPOINT cockroach_restart"); err != nil {

testserver: support windows

There are lots of unix assumptions here at the moment. For the 1.0 release, it would be fantastic to call out the existence of this package as a quick way of getting started with CRDB in Go projects, and we should be sure that we support the major platforms.

cc @mberhault @nvanbenschoten @BramGruneir @spencerkimball

pgx: automatic prepared statements

Does cockroachDB support prepared statement and is it beneficial to use them?
pgx library creates prepared statements and cache them per connection by default: docs
Is this behaviour totally fine for cockroachDB?

testserver allows connections before db "system" table is created

This will cause tests to sporadically fail because cockroach receives SQL to create new database / table before it's ready.

Upgrade GORM helper to use GORM v2

We are using https://github.com/jinzhu/gorm which is not maintained now.

The project is now at https://github.com/go-gorm/gorm

standard package errors should be used instead of archived pkg/errors

Hi,

In https://github.com/cockroachdb/cockroach-go/blob/master/testserver/version/version.go the archived pkg/errors is still being used, it should be replaced with fmt.Errorf().

transaction still told to restart when using ExecuteTx.

Running the latest photos app (8f188105e3cc5e9b2a82d63e8f7c4e2aeecbec4a) against the beta cluster, we're seeing fewer, but still some retriable transactions:

I160328 19:52:42.872224 db.go:202  insert into photos failed: pq: restart transaction: txn "sql/executor.go:380 sql txn" id=0bc74e29 key=/Table/53/2/""/Mon Mar 28 19:52:42 UTC 2016/0 rw=false pri=0.03138632 iso=SERIALIZABLE stat=PENDING epo=2 ts=1459194762.831141491,18 orig=1459194762.831141491,18 max=1459194763.081141491,0 wto=false failed to push "sql/executor.go:380 sql txn" id=e37df20a key=/Table/53/2/""/Mon Mar 28 19:52:42 UTC 2016/0 rw=true pri=0.01630113 iso=SERIALIZABLE stat=PENDING epo=0 ts=1459194762.831141491,17 orig=1459194762.831141491,17 max=1459194762.839783929,0 wto=false

Compilation error

Hi,
It looks like jackc/pgx has been updated and cockroach-go won't compile anymore.
The patch may just be to replace pgx.PgError by *pgconn.PgError

Cheers,

--- a/crdb/tx.go
+++ b/crdb/tx.go
@@ -20,7 +20,7 @@ import (
        "context"
        "database/sql"
 
-       "github.com/jackc/pgx"
+       "github.com/jackc/pgconn"
        "github.com/lib/pq"
 )
 
@@ -143,7 +143,7 @@ func errCode(err error) string {
        case *pq.Error:
                return string(t.Code)
 
-       case pgx.PgError:
+       case *pgconn.PgError:
                return t.Code
 
        default:

testserver: Start() is useless

All constructors of the TestServer call Start() before returning. Start() cannot be used to restart a server. This is true of the multi-tenant server too. Why do we have this method? Also, why do we even have an interface for TestServer given there is just one impl?

Cannot be used in conjunction with github.com/lib/pq

Attempting to use this for a project where I'm hoping to also support postgres as well as cockroach db. However, since this package imports "github.com/cockroachdb/pq" it's impossible to also register the "github.com/lib/pq" postgres driver. The registration panics because they both want to be the "postgres" driver.

panic: sql: Register called twice for driver postgres

goroutine 1 [running]:
panic(0x714d60, 0xc420010d50)
    /usr/local/go/src/runtime/panic.go:500 +0x1a1
database/sql.Register(0x76b2ba, 0x8, 0xabda60, 0xaefe88)
    /usr/local/go/src/database/sql/sql.go:45 +0x1b2
github.com/lib/pq.init.1()
    /home/eric/src/github.com/lib/pq/conn.go:44 +0x5c
github.com/lib/pq.init()
    /home/eric/src/github.com/lib/pq/user_posix.go:25 +0x62f
github.com/coreos/dex/storage/sql.init()

What's the reason for using the fork? Is there a plan to get any required patches back into the upstream repo?

testserver: concurrent downloads

I have a question about how testserver is handling the concurrent downloading of the cockroach binary. As I understand through reading the code:
It makes a get request for the latest build, it retrieves the file name and builds the local path where it will save the file.
If local file already exists it does nothing, but if it's not, it creates that file and starts downloading into it.
What happens if two process will try to initiate a testserver. They both check the local file, it doesn't exist, and they both will try to create it and start write to it.
I think this situation is only possible in ci/cd server when you have multiple concurrent test stages and they are not isolated via Docker.

Custom binary - tests failure

I'm launching a cockroachdb docker container using image from https://hub.docker.com/r/cockroachdb/cockroach.
Then i'm building and testing cockroach-go:

#!/bin/bash -ex

# Dependency installation
microdnf install -y git wget make gcc-c++

#Set variables
PACKAGE_URL=https://github.com/cockroachdb/cockroach-go
#PACKAGE_VERSION is configurable can be passed as an argument.
#PACKAGE_VERSION=${1:-e0a95dfd547c}
PACKAGE_VERSION=${1:-v2.2.8}
PACKAGE_NAME=cockroach-go
OS_NAME=$(cat /etc/os-release | grep ^PRETTY_NAME | cut -d= -f2)

#Install go package and update go path if not installed
GO_VERSION=1.16.1
GO_ARCH=ppc64le
if ! command -v go &> /dev/null
then
curl -O https://dl.google.com/go/go$GO_VERSION.linux-$GO_ARCH.tar.gz
tar -C /usr/local -xzf go$GO_VERSION.linux-$GO_ARCH.tar.gz
export GOROOT=/usr/local/go
export GOPATH=$HOME/go
export PATH=$GOROOT/bin:$GOPATH/bin:$PATH
export GO111MODULE=auto
rm -f go$GO_VERSION.linux-$GO_ARCH.tar.gz
fi

# Check if package exists
if [ -d $PACKAGE_NAME ] ; then
  rm -rf $PACKAGE_NAME
  echo "$PACKAGE_NAME  | $PACKAGE_VERSION | $OS_NAME | GitHub | Removed existing package if any"
fi

# Download the repos
git clone $PACKAGE_URL

# Build and Test
cd $PACKAGE_NAME
git checkout $PACKAGE_VERSION
ret=$?
if [ $ret -eq 0 ] ; then
 echo "---------------------------$PACKAGE_VERSION found to checkout--------------------- "
else
 echo "---------------------------$PACKAGE_VERSION not found-----------------------------"
 exit
fi

# Ensure go.mod file exists
[ ! -f go.mod ] && go mod init $PACKAGE_NAME
go mod tidy

if ! go build -v ./...; then
    echo "------------------$PACKAGE_NAME: build failed-------------------------"
    exit 1
fi

export COCKROACH_BINARY=/cockroach/cockroach
$COCKROACH_BINARY --version
if ! go test -v ./...; then
    echo "------------------$PACKAGE_NAME: test failed-------------------------"
    exit 1
else
    echo "$PACKAGE_NAME  |  $PACKAGE_VERSION | GitHub  | Pass |  Both_Build_and_Test_Success" 
    exit 0
fi

The tests don't pass consistently.
Similar failures are observed on ppc64le platform.

Go modules version is broken

Hi! Sorry to bother you again :) But the modules version isn't go getable anymore:

go get  github.com/cockroachdb/cockroach-go/v2

Returns an error:

go get github.com/cockroachdb/cockroach-go/v2: github.com/cockroachdb/cockroach-go/[email protected]: invalid version: missing github.com/cockroachdb/cockroach-go/go.mod and .../v2/go.mod at revision v2.0.1

As I understand this happens because tag v2.0.1 is set on commit from the master, not the version-module branch:
https://github.com/cockroachdb/cockroach-go/tree/v2.0.1
And repository at this commit doesn't contain any information about Go modules.

Error parsing headers when downloading binary

I'm getting an error downloading the binary when trying to start up a test server

_, err := testserver.NewTestServer()

if err != nil {
  panic(err)
}

GET https://edge-binaries.cockroachdb.com/cockroach/cockroach.darwin-amd64.LATEST
Failed to fetch latest binary: error parsing Content-Disposition headers map[Accept-Ranges:[bytes] Age:[0] Alt-Svc:[h3=":443"; ma=2592000,h3-29=":443"; ma=2592000] Cache-Control:[no-cache,public,max-age=3600] Content-Length:[185562128] Content-Type:[application/octet-stream] Date:[Wed, 28 Dec 2022 20:23:01 GMT] Etag:["d2e43af9be2469a905d71c777b6b7727"] Last-Modified:[Wed, 28 Dec 2022 19:00:16 GMT] Server:[UploadServer] X-Goog-Generation:[1672254016424452] X-Goog-Hash:[crc32c=mC0r2g== md5=0uQ6+b4kaakF1xx3e2t3Jw==] X-Goog-Metageneration:[2] X-Goog-Storage-Class:[STANDARD] X-Goog-Stored-Content-Encoding:[identity] X-Goog-Stored-Content-Length:[185562128] X-Guploader-Uploadid:[ADPycdthy1breMpK_xtwPITgJN4EZMo0Hks_2Mfstr71vWpBFY2efhDwmzSJEci5ak8DfePemBLOI9J2Nb7rvUw9-x9c0Q]]: mime: no media type, attempting to use cockroach binary from your PATH

Looking at the code, it seems like it's trying parse a filename from the content-disposition header. But looking at the headers for that URL, it seems like there's no content-disposition header? I'm not sure if something changed on the cockroach db server side? Or maybe there's something that I'm doing wrong here

Feature Request: Support `BEGIN PRIORITY X` for pgx

Currently there is no way to specify the priority of a transaction when using the pgx integration on initialization of the transaction. This results in another round trip to the DB with a SET TRANSACTION PRIORITY.

It would be stellar if there was an option in crdbpgx.ExecuteTx() that would allow specification of a transaction priority.

ExecuteInTx does not honor context cancellation

The retry loop in ExecuteInTx does not check ctx.Done to abort the retries if the context is cancelled.

Found while working on adding query cancellation in the crdb SQL shell.

cc @andreimatei @rafiss

Feature: support go-pg/pq

Since pq uses pg.DB instead of sql.DB, the crdb package is unable to be used with pg.

Question about the use of crdb.Execute() function.

crdb package has that function:

cockroach-go/crdb/tx.go

Lines 95 to 102 in 73ffeee

	func Execute(fn func() error) (err error) {
	for {
	err = fn()
	if err == nil || !errIsRetryable(err) {
	return err
	}
	}
	}

From the docs it's clear that it should be used to retry single statement (implicit transaction) operations.
But I don't quite understand should my app use it or not if I can't predict is 16K result buffer is enough for me in all situations. Let me explain:
My application will extract reasonable amount of rows up to 100 e.g. via limits and it's not going to stream data somewhere else. So it scans data from all rows in an array locally and return as a whole.
But I can't be sure that some batch won't exceed the 16K limit (for example because of long texts in some column).
To protect myself from transaction contention errors. I see two solutions here:

1. I could wrap all my single statement calls to cockraochDB in crdb.Execute().
2. Or It's better to increase the result buffer to always be in the limit and don't allow cockroachdb to start to stream. And If I see transaction contention errors in the logs it will mean that I either should increase the limit again or investigate why my app extracts that much data and restrict it.

It might be unrelated to this repository and should be asked in the main slack channel.

pgx support

My issue is connected to #16.
I faced the issue that none of our transactions was retried.
During the investigation, we realized, that in this commit was added the dependency to lib/pq and it handles only errors arisen by pq driver, but we use jackc/pgx. I think we all know, that it's quite popular.
And it isn't clear from docs that there is support only for lib/pq.

Maybe we could find the solution which will be driver independent?

pq library alternatives.

As I can see from your docs, you propose to use the pq library to connect to Cockroachdb from the application.
After looking at this library I found several major issues in my opinion:

1. It doesn't support read/write timeouts, only connection timeout. So if we get a connection from the pool and network failed the query will hang because we don't have read timeouts.
   I found an open issue for this problem: lib/pq#450 it was opened 4 years ago and they still didn't do anything. Another guy did a PR lib/pq#792 but maintainers didn't react on it in a year.
   Another postgers library supports such timeouts: https://github.com/go-pg/pg

2. Another issue with the pq, is that if you want to timeout your queries via the context cancel mechanism, it will try to send a cancel request to the server and if it's unreachable it will, again, hang. Here is the opened issue in this regard (3 years old):
   lib/pq#620

I think it goes against the highly available and distributed nature of the CockroachDB.
Maybe there are some side car proxies that one can use to offload this timeout logic from the library. Like envoy for http.
So that is your advice in that regard ?

should be able to `return tx.Commit()` from closure?

sql: Transaction has already been committed or rolled back

        if err := crdb.ExecuteTx(client.DB, func(tx *sql.Tx) error {
            // do stuff

            return tx.Commit() // works when returning `nil`
        }); err != nil {
            t.Fatalf("%+v", err)
        }

create a version of ExecuteTx() that takes a sql.Conn instead of sql.DB

...for newer versions of Go that support it.

It'd help with cockroachdb/cockroach#23558, and generally when clients care what connection their txn runs on.

allow concurrent tests with testserver when binary is being downloaded

if two tests using the testserver are running concurrently (often when testing multiple packages), we may end up with one test starting the download, and another thinking the file is there (we only look for filenames).
This obviously causes the second test to fail since the binary file is still busy.

Possible solutions:

• tmp file names for the binary: kindda crappy, we'd have to download it for every single test
• tmp file that gets renamed: the binary would still get downloaded as many times as it takes for the first test to finish.
• lock file until the download is finished: would make all the other tests wait and only download once. any issues if the downloader dies? the test fails anyway.
• initOnce statement somewhere to download: this works if the test runner is a go binary that's running all tests, otherwise not so much.

Unit tests with sqlproxy fail against bleeding edge CockroachDB version

To repro:

go test -p 1 -v ./... -run TestRunServer/SecureTenantThroughProxy

# or to test with a specific CRDB binary
COCKROACH_BINARY=/path/to/binary go test -p 1 -v ./... -run TestRunServer/SecureTenantThroughProxy

output

=== RUN   TestRunServer/SecureTenantThroughProxy
2021/06/04 17:14:50 GET https://edge-binaries.cockroachdb.com/cockroach/cockroach.darwin-amd64.LATEST
2021/06/04 17:14:50 Using automatically-downloaded binary: /var/folders/1_/_kcb28_94w16ml515w877ynm0000gn/T/cockroach.darwin-amd64.37fdb96e18cce4f11abc6e0fec5c74a770c65054
2021/06/04 17:14:51 process 44191 started: /var/folders/1_/_kcb28_94w16ml515w877ynm0000gn/T/cockroach.darwin-amd64.37fdb96e18cce4f11abc6e0fec5c74a770c65054 start-single-node --logtostderr --certs-dir=/tmp/cockroach-testserver239492345/certs --host=localhost --port=0 --http-port=0 --store=/tmp/cockroach-testserver239492345 --listening-url-file=/tmp/cockroach-testserver239492345/listen-url
2021/06/04 17:14:58 process 44261 started: /var/folders/1_/_kcb28_94w16ml515w877ynm0000gn/T/cockroach.darwin-amd64.37fdb96e18cce4f11abc6e0fec5c74a770c65054 mt start-sql --certs-dir=/tmp/cockroach-testserver239492345/certs --logtostderr --tenant-id=2 --kv-addrs=localhost:53249 --sql-addr=localhost:53267
2021/06/04 17:14:58 WaitForInit: Trying again after error: dial tcp [::1]:53267: connect: connection refused
2021/06/04 17:14:59 WaitForInit: Trying again after error: dial tcp [::1]:53267: connect: connection refused
2021/06/04 17:14:59 Process 44261 exited with status 1: exit status 1
2021/06/04 17:14:59 exit status 1
2021/06/04 17:14:59 WaitForInit: Trying again after error: dial tcp [::1]:53267: connect: connection refused

It looks like these are the changes in CockroachDB that could have caused the regression:

❯ git lg pkg/ccl/sqlproxyccl/
* 6d024aef05 - ccl/sqlproxyccl: CC code migration to DB (8 days ago) <Darin Peshev>
* 5a7d90c8fa - bazel: add lint to ensure new broken tests aren't added (9 days ago) <Ricky Stewart>
*   c8a62f290a - Merge #64957 (3 weeks ago) <craig[bot]>
|\
| * 935045a68a - sqlproxyccl: add support for expiring a denylist entry (3 weeks ago) <Ye Ji>
* | 15709c652d - bazel: handle `mockgen` in Bazel sandbox (3 weeks ago) <Ricky Stewart>
|/
* c5ca67d09f - ccl/sqlproxyccl: tenant denylist in yml format (4 weeks ago) <Darin Peshev>
*-.   051de519bd - Merge #64305 #64446 #64497 (5 weeks ago) <craig[bot]>
|\ \
* | | 1176126dc5 - ccl/sqlproxyccl: fix a broken test in bazel (5 weeks ago) <Darin Peshev>
| |/
|/|
| * 8bd62abe82 - ccl/sqlproxyccl: add basic login rate limiting (5 weeks ago) <Darin Peshev>
|/
* 65c30a3863 - ccl/sqlproxyccl: directory proto and test server (5 weeks ago) <Darin Peshev>
* c2f364a4d7 - ccl/sqlproxyccl: connection cache (5 weeks ago) <Darin Peshev>

Note that TestRunServer/SecureTenant passes, so it seems like sqlproxy is related.

cc @darinpp, do you think you'd be able to investigate further?

Sanitize cockroach-go repo

@andreimatei has informed us of some decisions that should be made about this repo and its organization. The full information is below, but TLDR: we should probably move testserver to its own repo, and consolidate the branches of this repo into one that uses modules.

This issue will document the decisions that we make.

The cockroach-go repo contains two top-level packages - crdb and testserver. crdb is a client library that implements a retry loop. testserver manages an out-of-process crdb server; I think that, as written, it's not very useful.

I've recently added the crdb/crdbpgx and crdb/crdbgorm packages, with retry-loops for the respective client libraries. The pgx one is a bit of a saga. Parts of the latest version of the pgx library only work when built with go modules. That's because pgx is supposed to be imported as github.com/jackc/pgx/v4. Subpackages such as pgxpool use this import path to refer to the parent package. So, I think, that the only way to use pgxpool is with modules.

The github.com/jackc/pgx package can still be imported without the v4 by an app not using modules. This is also how the main branch of crdbpgx is importing it. But this import path is no longer valid as soon as you start importing modules; you have to use the /v4 one. Confusingly, as soon as you start compiling code with modules enabled, the github.com/jackc/pgx import path starts resolving as the v3 of pgx (because of how tags are set up in their repo? or something), and chaos follows.

So, you're not using pgxpool, and you're not using modules in your client app, you can still use the main parts of pgx and the crdbpgx package. That's what I'm intending to do for cockroach's vendored copy of crdbpgx and pgx. If, however, a client app is using modules, then you can't use crdbpgx as is any more (*). Because somebody was on my case about it the other day, what Andrew W and I came up with was creating a separate branch of the repo, adding module support (go.mod), and changing all the import paths to pgx/v4. Then I've tagged the tip of that branch as v2.0.0, and so now you can import a modules-compatible crdb (and crdbpgx) package as github.com/cockroachdb/cockroach-go/v2/crdb/...
You can continue to import github.com/cockroachdb/cockroach-go/crdb, which refers to the non-modules compatible master branch.

Is that the right thing to do? I don't know. The branch, and the v2 import path, apply to both crdb and testserver. This seems bad, since I don't think we want to version testserver the same as crdb. I think we could have the versioning only apply to crdb (so, the import path would be github.com/cockroachdb/cockroach-go/crdb/v2/...) as go modules do have support for multiple modules in the same repo. However, it seems to be pretty discouraged and comes with caveats that are taking me too much time to understand. I'm thinking it might be better to move testserver to a different repo (and generally to give it some sprucing up).
In any case the branching generally seems very unfortunate to me.

go test ./... fails with -cockroach-binary flag

I've run into the following problem:
I want to provide a custom binary to the testserver package and I do it like that:
go test ./... -cockroach-binary cockroach
It fails with the following error:
flag provided but not defined: -cockroach-binary
As I understand the situation is this:
I run tests in all packages inside my project, most of my packages knows nothing about the testserver package and doesn't import it and hence the cockroach-binary flag isn't defined for them. Packages that do use testserver pass successfully. And when I run go test -cockroach-binary cockroach directly inside one those packages I don't see that error at all.

I think an environment variable is a better choice for that. We can look it up alongside with the flag to not break the compatibility. Alternatively, we could add a new function that receives the binary name:
func NewTestServerWithBinary(binary string) (*TestServer, error)

I could open a PR if you agree with that.

Support Go 1.13 error wrapping in ExecuteTx

Go 1.13 has built in support for error wrapping and unwrapping: https://golang.org/pkg/errors/
There is a transitional package at https://godoc.org/golang.org/x/xerrors

Problem: When using ExecuteTx, errors returned from the user supplied function must wrap the underlying error correctly, so that the transaction can be retried. This worked well previously as long as you implemented the ErrorCauser interface or use pkg/errors to wrap the underlying error.

In Go 1.13, to wrap errors, there is a %w placeholder: fmt.Errorf("error doing something: %w", err). You can then use errors.Unwrap() to retrieve the underlying error, use Is to assert the error type or use As to retrieve the error if it matches a type.

I think this package should be updated to support Go 1.13 error wrapping in addition to the current ErrorCauser interface as error wrapping using the stdlib's errors package is the recommended way, moving forward. We are currently in the process of migrating to Go 1.13 and are dropping pkg/errors for stdlib errors where possible.

testserver: provide more control over store like directory or in-mem

Today we run the testserver using a temp dir rooted in /tmp. I imagine there are cases where that choice of tempdir and ignoring TEMPDIR is a problem for some users. For other users, running an in-memory store would be preferable.

pgx compatibility

In my previous issue I was advised to use CockroachDB with the pgx library. And I started to look deeper into it.
I found out that pgx.Tx isn't compatible with the crdb.Tx interface and I can't use the crdb.ExecuteInTx() function.
I found a wrapper in the main cockroach repo: here
But it uses very old version of the library, you can see it in the import statement:
import "github.com/jackc/pgx"
and the current import path is "github.com/jackc/pgx/v4"
The wrapper wasn't updated in 2 years and calls methods that no longer exist in pgx.

Aside from this incompatibility issue, wouldn't it be better to put those helpers in this repo, instead of the main server repo?

add support for sqlx

I was looking to use sqlx library with crdb client for transactions, but noticed there was no support for github.com/jmoiron/sqlx.

This issue is to track sqlx compatibility for crdb.

testserver: download releases, not master builds

cockroach-go/testserver currently downloads the latest build from the master branch. It should be downloading the latest release instead.

testserver: Make it easier to use store=type=mem

Currently it defaults to an on-disk store in a tmp directory, but for many testing use-cases an in-memory store would be better.

Prevent transaction from committing on panic.

Currently there is no check for panics within the ExecTx closure. I think it could be useful to have a deferred function rollback the transaction if there is a panic.

Testing in pipeline

I'm trying to run a test with this server at Gitlab in Pipeline. As always everything works locally but at Gitlab I got

2023/01/11 16:21:06 cockroach-go testserver: [ldd --version]: out="" err=exit status 1
2023/01/11 16:21:06 GET https://binaries.cockroachdb.com/cockroach-v22.1.12.linux-amd64.tgz
2023/01/11 16:21:07 saving https://binaries.cockroachdb.com/cockroach-v22.1.12.linux-amd64.tgz to /tmp/cockroach-22.1.12, this may take some time
2023/01/11 16:21:09 Using automatically-downloaded binary: /tmp/cockroach-22.1.12
{"level":"fatal","msg":"cockroach-go testserver command /tmp/cockroach-22.1.12 version failed: exit status 127","time":"2023-01-11T16:21:09Z"}

small part of code

...
ts, err := testserverCR.NewTestServer(testserverCR.CustomVersionOpt("22.1.12"))
	if err != nil {
		log.Fatal(err)
	}
...