roachtest: add SQL instance to connection options#111582
roachtest: add SQL instance to connection options#111582craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
srosenberg
left a comment
There was a problem hiding this comment.
At the moment all callers are passing 0. Is there an imminent use-case where it would be non-zero?
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @herkolategan and @smg260)
pkg/cmd/roachtest/tests/drain.go line 246 at r1 (raw file):
prepareCluster(ctx, t, c, drainWaitDuration, connectionWaitDuration, queryWaitDuration) pgURL, err := c.ExternalPGUrl(ctx, t.L(), c.Node(nodeToDrain), "" /* tenant */, 0)
Nit: /* sqlInstance */ 0 [1]
renatolabs
left a comment
There was a problem hiding this comment.
Reviewable status:
Previously `ConnOption` did not support passing a SQL Instance. This change adds SQL Instance to `ConnOption` and updates the relevant calls to pass the information in order to support specifying a specific SQL instance if required. A SQL Instance can be viewed as a unique identifier for the SQL Instance that is running on a VM in the event that there are more than one SQL Instance serving the same virtual cluster on that VM. In the future virtual cluster name and SQL instance could be grouped possibly in struct for convenience. Epic: None Release Note: None
a4c7d47 to
7de3b43
Compare
herkolategan
left a comment
There was a problem hiding this comment.
At the moment all callers are passing
0. Is there an imminent use-case where it would be non-zero?
At the moment there is oneroachtestwhich will use this to start multiple instances on a single VM of the same virtual cluster. But since it's used scarcely, we have discussed using a struct for virtual cluster name and instance to alleviate that.
Reviewable status:
pkg/cmd/roachtest/tests/drain.go line 246 at r1 (raw file):
Previously, srosenberg (Stan Rosenberg) wrote…
Nit:
/* sqlInstance */ 0[1]
Done.
|
TFTRs! |
|
Build succeeded: |
Previously
ConnOptiondid not support passing a SQL Instance. This change adds SQL Instance toConnOptionand updates the relevant calls to pass the information in order to support specifying a specific SQL instance if required.A SQL Instance can be viewed as a unique identifier for the SQL Instance that is running on a VM in the event that there are more than one SQL Instance serving the same virtual cluster on that VM.
In the future virtual cluster name and SQL instance could be grouped possibly in struct for convenience.
Epic: None
Release Note: None