Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
75169: vendor: pull in latest version of `stress` r=rail a=rickystewart

Pull in the latest version of `stress` including these changes:

```
43d99a9 Merge pull request #13 from cockroachdb/bazelsharding
01690a1 stress: add `-bazel` support, support for sharding artifacts
```

Release note: None

75194: cli: stop ignoring user arg in insecure mode r=otan,knz a=rafiss

fixes #74704

Release note (bug fix): The --user argument is no longer ignored when
using `cockroach sql` in --insecure mode.

75212: kvserver: de-flake TestReplicateQueueUpAndDownReplicateNonVoters r=irfansharif a=irfansharif

Fixes #75135. This test asserted on span configs applying to a scratch
range. When stressing, it appeared that some time we were not seeing the
scratch range adopt the prescribed number of voters/non-voters. Staring
at the test itself, we were only nudging the replication queues for the
first node in the three node test. It's possible for the scratch range
to have been housed on a node other than the first; this commit makes it
so that the test nudges queues on all nodes. For good measure, lets also
ensure that the split queues process everything, ditto for the snapshot
queues.

To repro:

    dev test pkg/kv/kvserver \
      -f TestReplicateQueueUpAndDownReplicateNonVoters \
      -v --show-logs --timeout 2m --stress

Release note: None

75218: backupccl: disable span configs for full cluster restore jobs test r=irfansharif a=RichardJCai

Release note: None

Related #75216

75220: kvserver: disable sendWithRangeID call stack r=knz a=tbg

Sadly, this on longer embeds the RangeID in the stack trace (likely
culprit: Go adopting a register-based calling convention) Instead, you
get garbage values that often are obviously garbage, but this may not
always be true. We want to avoid being misled, so for now remove the
rangeID here and explain when it can come back.

See: https://cockroachlabs.slack.com/archives/G01G8LK77DK/p1641478596004700

Release note: None


Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: richardjcai <caioftherichard@gmail.com>
Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
  • Loading branch information
6 people committed Jan 20, 2022
6 parents ebda0ec + 9e91b5a + 9b3efa2 + ee1e9cd + 9c6e09b + bd72f75 commit 9e22d9d
Show file tree
Hide file tree
Showing 13 changed files with 49 additions and 40 deletions.
6 changes: 3 additions & 3 deletions DEPS.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -1258,10 +1258,10 @@ def go_deps():
name = "com_github_cockroachdb_stress",
build_file_proto_mode = "disable_global",
importpath = "github.com/cockroachdb/stress",
sha256 = "2b7b584a4cafacd0d971adf1e150fe9c1f770eb2b56993af06a4ae7fa329a521",
strip_prefix = "github.com/cockroachdb/stress@v0.0.0-20170808184505-29b5d31b4c3a",
sha256 = "bd0dea0ebea9ea71aa12e5918fa8b61a78a548d54e8e9c126aa285c1ae84d3e4",
strip_prefix = "github.com/cockroachdb/stress@v0.0.0-20220119200057-43d99a9e6d7f",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/stress/com_github_cockroachdb_stress-v0.0.0-20170808184505-29b5d31b4c3a.zip",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/stress/com_github_cockroachdb_stress-v0.0.0-20220119200057-43d99a9e6d7f.zip",
],
)
go_repository(
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ require (
github.com/cockroachdb/redact v1.1.3
github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
github.com/cockroachdb/sentry-go v0.6.1-cockroachdb.2
github.com/cockroachdb/stress v0.0.0-20170808184505-29b5d31b4c3a
github.com/cockroachdb/stress v0.0.0-20220119200057-43d99a9e6d7f
github.com/cockroachdb/tools v0.0.0-20211112185054-642e51449b40
github.com/cockroachdb/ttycolor v0.0.0-20210902133924-c7d7dcdde4e8
github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,8 @@ github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd h1:KFOt5I9
github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd/go.mod h1:AN708GD2FFeLgUHMbD58YPe4Nw8GG//3rwgyG4L9gR0=
github.com/cockroachdb/sentry-go v0.6.1-cockroachdb.2 h1:IKgmqgMQlVJIZj19CdocBeSfSaiCbEBZGKODaixqtHM=
github.com/cockroachdb/sentry-go v0.6.1-cockroachdb.2/go.mod h1:8BT+cPK6xvFOcRlk0R8eg+OTkcqI6baNH4xAkpiYVvQ=
github.com/cockroachdb/stress v0.0.0-20170808184505-29b5d31b4c3a h1:7bnDlMxIJCg3o3vIILG2COsbNZpiYHgI4UkCYmeAWnQ=
github.com/cockroachdb/stress v0.0.0-20170808184505-29b5d31b4c3a/go.mod h1:oapRqABg5KA/TaAikQH53fpP5nvCe9sftYmYV/F/yVE=
github.com/cockroachdb/stress v0.0.0-20220119200057-43d99a9e6d7f h1:jXa4+uPllulalBUwZlptnW177YokSScljz6TpsMGld8=
github.com/cockroachdb/stress v0.0.0-20220119200057-43d99a9e6d7f/go.mod h1:oapRqABg5KA/TaAikQH53fpP5nvCe9sftYmYV/F/yVE=
github.com/cockroachdb/tablewriter v0.0.5-0.20200105123400-bd15540e8847 h1:c7yLgqcm/3c9lYtpWeVD9NYqA9cKsKHdpQM62PHtTUM=
github.com/cockroachdb/tablewriter v0.0.5-0.20200105123400-bd15540e8847/go.mod h1:zq6QwlOf5SlnkVbMSr5EoBv3636FWnp+qbPhuoO21uA=
github.com/cockroachdb/teamcity v0.0.0-20180905144921-8ca25c33eb11 h1:UAqRo5xPCyTtZznAJ9iPVpDUFxGI0a6QWtQ8E+zwJRg=
Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/backupccl/full_cluster_backup_restore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func TestFullClusterBackup(t *testing.T) {

params := base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
DisableSpanConfigs: true, // TODO(irfansharif): #75060.
Knobs: base.TestingKnobs{
SpanConfig: &spanconfig.TestingKnobs{
// We compare job progress before and after a restore. Disable
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/democluster/demo_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -1147,7 +1147,8 @@ func (c *transientCluster) getNetworkURLForServer(
host, port, _ := addr.SplitHostPort(sqlAddr, "")
u.
WithNet(pgurl.NetTCP(host, port)).
WithDatabase(database)
WithDatabase(database).
WithUsername(c.adminUser.Normalized())

// For a demo cluster we don't use client TLS certs and instead use
// password-based authentication with the password pre-filled in the
Expand All @@ -1156,7 +1157,6 @@ func (c *transientCluster) getNetworkURLForServer(
u.WithInsecure()
} else {
u.
WithUsername(c.adminUser.Normalized()).
WithAuthn(pgurl.AuthnPassword(true, c.adminPassword)).
WithTransport(pgurl.TransportTLS(pgurl.TLSRequire, ""))
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/interactive_tests/test_style_enabled.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ eexpect eof

# Try connect and check the session variables match.

spawn $argv sql --url "postgresql://test@localhost:26257?options=-cintervalstyle%3Diso_8601"
spawn $argv sql --url "postgresql://root@localhost:26257?options=-cintervalstyle%3Diso_8601"
eexpect root@
send "SHOW intervalstyle;\r"
eexpect "iso_8601"
Expand All @@ -28,7 +28,7 @@ interrupt
eexpect eof

# TODO(#72065): uncomment
#spawn $argv sql --url "postgresql://test@localhost:26257?options=-cdatestyle%3Dymd"
#spawn $argv sql --url "postgresql://root@localhost:26257?options=-cdatestyle%3Dymd"
#eexpect root@
#send "SHOW datestyle;\r"
#eexpect "ISO, YMD"
Expand Down
5 changes: 5 additions & 0 deletions pkg/cli/interactive_tests/test_url_db_override.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ spawn $argv sql --url "postgresql://test@localhost:26257" --insecure -e "select
eexpect "1 row"
eexpect eof

# Make sure --insecure does not override --user
spawn $argv sql --user=test --insecure -e "select 'user=' || current_user()"
eexpect "user=test"
eexpect eof

set ::env(COCKROACH_INSECURE) "true"
end_test

Expand Down
15 changes: 12 additions & 3 deletions pkg/kv/kvserver/replica_send.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ var optimisticEvalLimitedScans = settings.RegisterBoolSetting(
func (r *Replica) Send(
ctx context.Context, ba roachpb.BatchRequest,
) (*roachpb.BatchResponse, *roachpb.Error) {
return r.sendWithRangeID(ctx, r.RangeID, &ba)
return r.sendWithoutRangeID(ctx, &ba)
}

// checkCircuitBreaker takes a cancelable context and its cancel function. The
Expand Down Expand Up @@ -163,6 +163,15 @@ func maybeAdjustWithBreakerError(pErr *roachpb.Error, brErr error) *roachpb.Erro
return pErr
}

// sendWithoutRangeID used to be called sendWithRangeID, accepted a `_forStacks
// roachpb.RangeID` argument, and had the description below. Ever since Go
// switched to the register-based calling convention though, this stopped
// working, giving essentially random numbers in the goroutine dumps that were
// misleading. It has thus been "disarmed" until Go produces useful values
// again.
//
// See (internal): https://cockroachlabs.slack.com/archives/G01G8LK77DK/p1641478596004700
//
// sendWithRangeID takes an unused rangeID argument so that the range
// ID will be accessible in stack traces (both in panics and when
// sampling goroutines from a live server). This line is subject to
Expand All @@ -174,8 +183,8 @@ func maybeAdjustWithBreakerError(pErr *roachpb.Error, brErr error) *roachpb.Erro
// within the portion printed in the stack trace):
//
// github.com/cockroachdb/cockroach/pkg/storage.(*Replica).sendWithRangeID(0xc420d1a000, 0x64bfb80, 0xc421564b10, 0x15, 0x153fd4634aeb0193, 0x0, 0x100000001, 0x1, 0x15, 0x0, ...)
func (r *Replica) sendWithRangeID(
ctx context.Context, _forStacks roachpb.RangeID, ba *roachpb.BatchRequest,
func (r *Replica) sendWithoutRangeID(
ctx context.Context, ba *roachpb.BatchRequest,
) (_ *roachpb.BatchResponse, rErr *roachpb.Error) {
var br *roachpb.BatchResponse
if r.leaseholderStats != nil && ba.Header.GatewayNodeID != 0 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/replica_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12667,7 +12667,7 @@ func TestProposalNotAcknowledgedOrReproposedAfterApplication(t *testing.T) {

// Round trip another proposal through the replica to ensure that previously
// committed entries have been applied.
_, pErr = tc.repl.sendWithRangeID(ctx, tc.repl.RangeID, &ba)
_, pErr = tc.repl.sendWithoutRangeID(ctx, &ba)
if pErr != nil {
t.Fatal(pErr)
}
Expand Down
31 changes: 13 additions & 18 deletions pkg/kv/kvserver/replicate_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,23 +301,22 @@ func TestReplicateQueueDownReplicate(t *testing.T) {
}

func scanAndGetNumNonVoters(
ctx context.Context,
t *testing.T,
tc *testcluster.TestCluster,
store *kvserver.Store,
scratchKey roachpb.Key,
t *testing.T, tc *testcluster.TestCluster, scratchKey roachpb.Key,
) (numNonVoters int) {
// Nudge the replicateQueue to up/down-replicate our scratch range.
if err := store.ForceReplicationScanAndProcess(); err != nil {
t.Fatal(err)
for _, s := range tc.Servers {
// Nudge internal queues to up/down-replicate our scratch range.
require.NoError(t, s.Stores().VisitStores(func(s *kvserver.Store) error {
require.NoError(t, s.ForceSplitScanAndProcess())
require.NoError(t, s.ForceReplicationScanAndProcess())
require.NoError(t, s.ForceRaftSnapshotQueueProcess())
return nil
}))
}
scratchRange := tc.LookupRangeOrFatal(t, scratchKey)
row := tc.ServerConn(0).QueryRow(
`SELECT array_length(non_voting_replicas, 1) FROM crdb_internal.ranges_no_leases WHERE range_id=$1`,
`SELECT coalesce(max(array_length(non_voting_replicas, 1)),0) FROM crdb_internal.ranges_no_leases WHERE range_id=$1`,
scratchRange.GetRangeID())
err := row.Scan(&numNonVoters)
log.Warningf(ctx, "error while retrieving the number of non-voters: %s", err)

require.NoError(t, row.Scan(&numNonVoters))
return numNonVoters
}

Expand All @@ -329,7 +328,6 @@ func TestReplicateQueueUpAndDownReplicateNonVoters(t *testing.T) {
skip.UnderRace(t)
defer log.Scope(t).Close(t)

ctx := context.Background()
tc := testcluster.StartTestCluster(t, 1,
base.TestClusterArgs{
ReplicationMode: base.ReplicationAuto,
Expand Down Expand Up @@ -360,13 +358,10 @@ func TestReplicateQueueUpAndDownReplicateNonVoters(t *testing.T) {
// Add two new servers and expect that 2 non-voters are added to the range.
tc.AddAndStartServer(t, base.TestServerArgs{})
tc.AddAndStartServer(t, base.TestServerArgs{})
store, err := tc.Server(0).GetStores().(*kvserver.Stores).GetStore(
tc.Server(0).GetFirstStoreID())
require.NoError(t, err)

var expectedNonVoterCount = 2
testutils.SucceedsSoon(t, func() error {
if found := scanAndGetNumNonVoters(ctx, t, tc, store, scratchKey); found != expectedNonVoterCount {
if found := scanAndGetNumNonVoters(t, tc, scratchKey); found != expectedNonVoterCount {
return errors.Errorf("expected upreplication to %d non-voters; found %d",
expectedNonVoterCount, found)
}
Expand All @@ -379,7 +374,7 @@ func TestReplicateQueueUpAndDownReplicateNonVoters(t *testing.T) {
require.NoError(t, err)
expectedNonVoterCount = 0
testutils.SucceedsSoon(t, func() error {
if found := scanAndGetNumNonVoters(ctx, t, tc, store, scratchKey); found != expectedNonVoterCount {
if found := scanAndGetNumNonVoters(t, tc, scratchKey); found != expectedNonVoterCount {
return errors.Errorf("expected downreplication to %d non-voters; found %d",
expectedNonVoterCount, found)
}
Expand Down
1 change: 0 additions & 1 deletion pkg/server/pgurl/pgurl.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ func (u *URL) GetOption(opt string) string {
// all security controls disabled.
func (u *URL) WithInsecure() *URL {
return u.
WithUsername("root").
WithAuthn(AuthnNone()).
WithTransport(TransportNone())
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/server/pgurl/testdata/url
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,13 @@ JDBC: jdbc:postgresql://somehost:26257/somedb?application_name=myapp&password=

insecure
----
pq URL: postgresql://root@/?sslmode=disable
DSN: user=root sslmode=disable
JDBC: jdbc:postgresql:///?sslmode=disable&user=root
pq URL: postgresql:///?sslmode=disable
DSN: sslmode=disable
JDBC: jdbc:postgresql:///?sslmode=disable
--defaults filled--
pq URL: postgresql://root@defaulthost:26257/defaultdb?sslmode=disable
DSN: database=defaultdb user=root host=defaulthost port=26257 sslmode=disable
JDBC: jdbc:postgresql://defaulthost:26257/defaultdb?sslmode=disable&user=root
pq URL: postgresql://defaultuser@defaulthost:26257/defaultdb?sslmode=disable
DSN: database=defaultdb user=defaultuser host=defaulthost port=26257 sslmode=disable
JDBC: jdbc:postgresql://defaulthost:26257/defaultdb?sslmode=disable&user=defaultuser

subtest redundant

Expand Down
2 changes: 1 addition & 1 deletion vendor

0 comments on commit 9e22d9d

Please sign in to comment.