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

roachtest: sstable-corruption/table failed #77321

Closed
cockroach-teamcity opened this issue Mar 3, 2022 · 10 comments
Closed

roachtest: sstable-corruption/table failed #77321

cockroach-teamcity opened this issue Mar 3, 2022 · 10 comments
Assignees
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. T-storage Storage Team

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Mar 3, 2022

roachtest.sstable-corruption/table failed with artifacts on master @ dbdbb93978a9ad564e07085c15807f415ccaa00a:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/sstable-corruption/table/run_1
	sstable_corruption.go:58,sstable_corruption.go:114,test_runner.go:866: Non-zero exit code: 1
Help

See: roachtest README

See: How To Investigate (internal)

/cc @cockroachdb/storage

This test on roachdash | Improve this report!

Jira issue: CRDB-13527

@cockroach-teamcity cockroach-teamcity added branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Mar 3, 2022
@blathers-crl blathers-crl bot added the T-storage Storage Team label Mar 3, 2022
@itsbilal itsbilal self-assigned this Mar 3, 2022
nicktrav added a commit to nicktrav/cockroach that referenced this issue Mar 3, 2022
To aid in debugging cockroachdb#77321, log the contents stdout and stderr if the
manifest dump command fails.

Release justification: Tests only.

Release note: None.
craig bot pushed a commit that referenced this issue Mar 3, 2022
…77328 #77335

75751: sql: Add DateStyle/IntervalStyle visitor r=e-mbrown a=e-mbrown

The DateStyle visitor allows for cast expressions with string
to interval and date/interval types to string cast to be rewritten.
These stable cast cause issues with DateStyle/IntervalStyle formatting so they
need to be wrapped in builtins containing their immutable version.

Release note: None
Release justification: Low risk update to new functionality

76705: backupccl: add prototype metadata.sst r=rhu713 a=rhu713

This adds writing of an additional file to the completion of BACKUP. This new file
is an sstable that contains the same metadata currently stored in the BACKUP_MANIFEST
file and statistics files, but organizes that data differently.

The current BACKUP_MANIFEST file contains a single binary-encoded protobuf message
of type BackupManifest, that in turn has several fields some of which are repeated
to contain e.g. the TableDescriptor for every table backed up, or every revision to
every table descriptor backed up. This can result in these manifests being quite large
in some cases, which is potentially concerning because as a single protobuf message,
one has to read and unmarshal the entire struct into memory to read any field(s) of it.

Organizing this metadata into an SSTable where repeated fields are instead stored as
separate messages under separate keys should instead allow reading it incrementally:
one can seek to a particular key or key prefix and then scan, acting on whatever data
is found as it is read, without loading the entire file at once (when opened using the
same seek-ing remote SST reader we use to read backup data ssts).

This initial prototype adds only the writer -- RESTORE does not rely on, or even open,
this new file at this time.

Release note: none.

77018: release: automate orchestration version update r=celiala a=rail

Previously, as a part of the release process we had to bump the
orchestration versions using `sed` with some error-prone regexes.

This patch adds `set-orchestration-version` subcommand to the release
tool. It uses templates in order to generate the orchestration files.

Release note: None

77055: sql: change index backfill merger to use batch api r=rhu713 a=rhu713

Use Batch API instead of txn.Scan() in order to limit the number of bytes per
batch response in the index backfill merger.

Fixes #76685.

Release note: None

77065: bazel: use test sharding more liberally r=rail a=rickystewart

Closes #76376.

Release note: None

77109: ccl/sqlproxyccl: add helpers related to connection migration r=JeffSwenson,andy-kimball a=jaylim-crl

#### ccl/sqlproxyccl: add helpers related to connection migration 

Informs #76000. Extracted from #76805.

This commit adds helpers related to connection migration. This includes support
for retrieving the transfer state through SHOW TRANSFER STATE, as well as
deserializing the session through crdb_internal.deserialize_session.

Release note: None

Release justification: Helpers added in this commit are needed for the
connection migration work. Connection migration is currently not being used
in production, and CockroachCloud is the only user of sqlproxy.
  
#### ccl/sqlproxyccl: fix math for defaultBufferSize in interceptors 

Previously, we incorrectly defined defaultBufferSize as 16K bytes. Note that
2 << 13 is 16K bytes. This commit fixes that behavior to match the original
intention of 8K bytes.

Release note: None

Release justification: This fixes an unintentional buglet within the sqlproxy
code that was introduced with the interceptors back then. Not having this in
means we're using double the memory for each connection within the sqlproxy.



77307: sql: add cluster setting to limit max size of serialized session r=otan,jaylim-crl a=rafiss

fixes #77302

The sql.session_transfer.max_session_size cluster setting can be used to
limit the max size of a session that is serialized using
crdb_internal.serialize_session.

No release note since this is not a public setting.

Release justification: high priority fix for new functionality.

Release note: None

77318: roachpb: extract keysbase to break some dependencies r=yuzefovich a=yuzefovich

This commit extracts a couple of things out of `roachpb` into new
`keysbase` package in order to break the dependency of `util/json` and
`sql/inverted` on `roachpb` (which is a part of the effort to clean up
the dependencies of `execgen`).

Addresses: #77234.

Release note: None

Release justification: low risk change to clean up the dependencies.

77319: sessiondatapb: move one enum definition into lex package r=yuzefovich a=yuzefovich

This commit moves the definition of `BytesEncodeFormat` enum from
`sessiondatapb` to `lex`. This is done in order to make `lex` not depend
on a lot of stuff (eventually on `roachpb`) and is a part of the effort
to clean up the dependencies of `execgen`. Note that the proto package
name is not changed, so this change is backwards-compatible.

Informs: #77234.

Release note: None

Release justification: low risk change to clean up the dependencies.

77328: roachtest: log stdout and stderr in sstable corruption test r=itsbilal a=nicktrav

To aid in debugging #77321, log the contents stdout and stderr if the
manifest dump command fails.

Release justification: Tests only.

Release note: None.

77335: kvserver: fix race that caused truncator to truncate non-alive replica r=tbg,erikgrinaker a=sumeerbhola

This was causing truncated state to be written to such a
replica, which would then get picked up as the
HardState.Commit value when a different replica was later
added back for the same range. See
#77030 (comment)
for the detailed explanation.

Also restore the default value of
kv.raft_log.loosely_coupled_truncation.enabled to true.

Fixes #77030

Release justification: Bug fix.
Release note: None

Co-authored-by: e-mbrown <[email protected]>
Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Rui Hu <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Jay <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Nick Travers <[email protected]>
Co-authored-by: sumeerbhola <[email protected]>
@cockroach-teamcity
Copy link
Member Author

roachtest.sstable-corruption/table failed with artifacts on master @ 0e1edf3ad9da29b1d3bb28fcfc0f293d700e3cb9:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/sstable-corruption/table/run_1
	sstable_corruption.go:58,sstable_corruption.go:114,test_runner.go:866: could not find tables to corrupt: Non-zero exit code: 1
		stdout: 
		stderr: 
Help

See: roachtest README

See: How To Investigate (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

roachtest.sstable-corruption/table failed with artifacts on master @ 71eb44ffb1574350cf4a36143ed423f6ac0ca02b:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/sstable-corruption/table/run_1
	sstable_corruption.go:58,sstable_corruption.go:114,test_runner.go:866: could not find tables to corrupt: Non-zero exit code: 1
		stdout: 
		stderr: 
Help

See: roachtest README

See: How To Investigate (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

roachtest.sstable-corruption/table failed with artifacts on master @ 3b7f761f019c0457c19726bab8d14c231cd318fc:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/sstable-corruption/table/run_1
	sstable_corruption.go:58,sstable_corruption.go:114,test_runner.go:866: could not find tables to corrupt: Non-zero exit code: 1
		stdout: 
		stderr: 
Help

See: roachtest README

See: How To Investigate (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

roachtest.sstable-corruption/table failed with artifacts on master @ c6a5a395cbecbbd5ba77c2c14e3bf68eb59eeb19:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/sstable-corruption/table/run_1
	sstable_corruption.go:58,sstable_corruption.go:114,test_runner.go:866: could not find tables to corrupt: Non-zero exit code: 1
		stdout: 
		stderr: 
Help

See: roachtest README

See: How To Investigate (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

roachtest.sstable-corruption/table failed with artifacts on master @ e7e596ede9d3da877e313a3d39ab498a5cb7d27b:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/sstable-corruption/table/run_1
	sstable_corruption.go:58,sstable_corruption.go:114,test_runner.go:866: could not find tables to corrupt: Non-zero exit code: 1
		stdout: 
		stderr: 
Help

See: roachtest README

See: How To Investigate (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

roachtest.sstable-corruption/table failed with artifacts on master @ 5755663effbdbe8175bb74e5cdc19b2298e0a0bc:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/sstable-corruption/table/run_1
	sstable_corruption.go:58,sstable_corruption.go:114,test_runner.go:866: could not find tables to corrupt: Non-zero exit code: 1
		stdout: 
		stderr: 
Help

See: roachtest README

See: How To Investigate (internal)

This test on roachdash | Improve this report!

nicktrav added a commit to nicktrav/cockroach that referenced this issue Mar 11, 2022
A few minor changes to the existing roachtest:

Reduce the warehouse count on ingest back to 100 - this should be ample,
and it matches the warehouse count on the read path in a later part of
the test.

Change the stop options to wait until the process has been terminated
before continuing.

Re-work the command to search for SSTables to corrupt - use tables from
the most recent manifest (if multiple are present); consider SSTables
with either a start or end key representing a user table; shuffle tables
to distribute corruption over the LSM; perform filtering in the bash
command rather than in the test runner itself.

Release justification: Roachtest-only change.

Release note: None.

Touches cockroachdb#77321.
@cockroach-teamcity
Copy link
Member Author

roachtest.sstable-corruption/table failed with artifacts on master @ 270eb7b3ab88fc47ba86d87f4e2113e560f047a8:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/sstable-corruption/table/run_1
	sstable_corruption.go:58,sstable_corruption.go:114,test_runner.go:866: could not find tables to corrupt: Non-zero exit code: 1
		stdout: 
		stderr: 
Help

See: roachtest README

See: How To Investigate (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

roachtest.sstable-corruption/table failed with artifacts on master @ dc5432d65b9c38ce1ce5832e3c49d345b66145dc:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/sstable-corruption/table/run_1
	sstable_corruption.go:58,sstable_corruption.go:114,test_runner.go:866: could not find tables to corrupt: Non-zero exit code: 1
		stdout: 
		stderr: 
Help

See: roachtest README

See: How To Investigate (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

roachtest.sstable-corruption/table failed with artifacts on master @ 231fd420c61e39b6ad08c982c496d82cf1910bc5:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /artifacts/sstable-corruption/table/run_1
	sstable_corruption.go:58,sstable_corruption.go:114,test_runner.go:866: could not find tables to corrupt: Non-zero exit code: 1
		stdout: 
		stderr: 
Help

See: roachtest README

See: How To Investigate (internal)

This test on roachdash | Improve this report!

@jbowens jbowens removed the release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. label Mar 14, 2022
craig bot pushed a commit that referenced this issue Mar 14, 2022
77442: sqlproxyccl: Add codeUnavailable to the list of error codes r=alyshanjahani-crl a=alyshanjahani-crl

Release justification: fixes for high-priority bug in existing functionality

Previously, if a tenant cluster had maxPods set to 0 the error returned by
directory.EnsureTenantAddr was not treated as a non-retryable error.

The tenant directory implementation used in CockroachCloud now identifies
this situation and returns a status error with codes.FailedPrecondition and
a descriptive message.

This patch adds a check for the FailedPrecondition error in
connector.lookupAddr.

Release note: None

77626: sql: re-enable txnidcache by default r=maryliag,ajwerner a=Azhng

Previously, TxnID cache was disabled by default due to performance
issues. This has since been resolved in #77220.
This commit re-enables TxnID Cache by default.

Release justification: low risk high reward change.
Release note: None

77675: cmd/roachtest: tweaks to sstable corruption test r=itsbilal a=nicktrav

A few minor changes to the existing roachtest:

Reduce the warehouse count on ingest back to 100 - this should be ample,
and it matches the warehouse count on the read path in a later part of
the test.

Change the stop options to wait until the process has been terminated
before continuing.

Re-work the command to search for SSTables to corrupt - use tables from
the most recent manifest (if multiple are present); consider SSTables
with either a start or end key representing a user table; shuffle tables
to distribute corruption over the LSM; perform filtering in the bash
command rather than in the test runner itself.

Release justification: Roachtest-only change.

Release note: None.

Touches #77321.

77700: ccl/sqlproxyccl: add connection migration-related metrics r=JeffSwenson a=jaylim-crl

#### ccl/sqlproxyccl: add metric that measures connection migration latency

Previously, we added support for connection migration in #76805. This commit
adds a new `proxy.conn_migration.attempted.latency` metric that tracks latency
for attempted connection migrations. Having this metric would be beneficial
as we can now know how long users were blocked during connection migrations.

Release justification: Low-risk metric-only change within sqlproxy.

Release note: None

#### ccl/sqlproxyccl: add metric that records the transfer response message size

Informs #76000, and follow up to #76805.

This commit adds a new proxy.conn_migration.transfer_response.message_size
metric that will track the distribution of the transfer response message size.
This will be used to tune a value for the SQL cluster setting:
sql.session_transfer.max_session_size.

Release justification: Low-risk metric-only change within sqlproxy.

Release note: None

Co-authored-by: Alyshan Jahani <[email protected]>
Co-authored-by: Azhng <[email protected]>
Co-authored-by: Nick Travers <[email protected]>
Co-authored-by: Jay <[email protected]>
@nicktrav
Copy link
Collaborator

After a number of consecutive failures, the test passed in last night's run, which included the change from #77675. By no means an indication that this is solved, but I'm going to close this issue out, so we get a new issue to investigate if / when it fires again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. T-storage Storage Team
Projects
None yet
Development

No branches or pull requests

4 participants