pkg/sql/copy/copy_test: TestLargeDynamicRows failed #109134
Assignees
Labels
branch-release-23.1
Used to mark GA and release blockers, technical advisories, and bugs for 23.1
C-test-failure
Broken test (automatically or manually discovered).
O-robot
Originated from a bot.
T-sql-queries
SQL Queries Team
Milestone
Comments
cockroach-teamcity
added
branch-release-23.1
|
This is an issue with test / test infra for small random values of sql.CopyBatchRowSize. Repro: |
craig bot
pushed a commit
that referenced
this issue
Aug 23, 2023
…109356 108485: github: code coverage workflows r=RaduBerinde a=RaduBerinde This change adds two GitHub Action workflows which run on each PR. One generates unit test code coverage data, and one publishes that data to a GCS bucket from where Reviewable can access it. We generate coverage data using `bazel coverage`, but we restrict it to only test the packages that have been modified by the PR. Two workflows are required for security (the first workflow runs potentially malicious code from a fork); for more details, see https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ Epic: none Release note: None 109036: roachprod: better determination if scp -R flag can be used r=RaduBerinde a=RaduBerinde When uploading a file to a cluster, we use the "tree dist" algorithm by default. This uploads the file to a single node, then we copy the file from that node to the other nodes (up to 10). This only makes sense if the remote-to-remote transfers can happen directly, which only happens if we pass the `-R -A` flags to `scp`. Unfortunately older versions don't support these flags. Currently the flags are only passed if the OS is `darwin`. This commits improves the determination - we run `ssh -V` (once) and check if the `SSL` major version is three. For reference, some examples of what `ssh -V` returns: - recent MacOSX: `OpenSSH_9.0p1, LibreSSL 3.3.6` - Ubuntu 22.04: `OpenSSH_8.9p1 Ubuntu-3ubuntu0.3, OpenSSL 3.0.2 15 Mar 2022` In addition, if the version is not 3, we disable the use of "tree dist". Epic: none Release note: None 109260: sql stats: skip tests hitting combinedstmts and statements endpoints r=gtr a=gtr Part of #109184. This commit skips tests which hit the `combinedStmts` or `statements` endpoints which will sometimes timeout under stress as a result of recent backend changes. The test investigation is tracked by #109184. Release note: None 109288: dev: error when trying to `dev test` a bazel tested target r=rickystewart a=liamgillies Running `dev test` on these integration tests will always fail, so this PR adds a error when running the command on those files. Fixes: #107813 Release note: None 109292: sql: fix expected batch count for edge case in copy test r=rharding6373 a=rharding6373 In TestLargeDynamicRows we test that 4 rows of data can fit in a batch size of at least 4 rows given default memory sizes. However, when we set the batch row size to the minimum value of 4, the test hook that counts batches counts an extra empty batch. This PR changes adjusts the minimum row size to 5 for the purposes of this test. Epic: None Fixes: #109134 Release note: None 109324: build: update bazel builder build docs r=rickystewart a=rail Previously, the documentation described a manual build of the `bazelbuilder` docker image. The current approach is to use CI to build the image. This PR updates the documentation to reflect the current process, including the FIPS image steps. Epic: none Release note: None 109340: changefeedccl: move node drain handling logic out of kvfeed r=miretskiy a=jayshrivastava Previously, the kvfeed was responsible for monitoring for node drains using a goroutine. This change moves this logic into the change aggregator and removes the goroutine. Overall, this change makes the code more organized and performant. This change was inspired by work being done for #109167. The work in that PR requires being able to restart the kvfeed. Having drain logic intermingled with the kvfeed makes restarts much more complex, hard to review, prone to bugs, etc. Informs: #96953 Release note: None Epic: None 109349: kv: wait on latches on each key in reverse acquisition order r=arulajmani,kvoli a=nvanbenschoten This commit allocates latch IDs from the top of the uint64 space and in reverse order. This is done to order latches in the tree on a same key in reverse order of acquisition. Doing so ensures that when we iterate over the tree and see a key with many conflicting latches, we visit the latches on that key in the reverse order that they will be released. In doing so, we minimize the number of open channels that we wait on (calls to `waitForSignal`) and minimize the number of goroutine scheduling points. This is important to avoid spikes in runnable goroutine after each request completes, which can negatively affect node health. See experiments below. Epic: None Release note (performance improvement): The impact of high concurrency blind writes to the same key on goroutine scheduling latency was reduced. 109356: build: explicitly set SKIP_LABEL_TEST_FAILURE in compose.sh r=rickystewart a=chrisseto Previously, `SKIP_LABEL_TEST_FAILURE` was being set via a teamcity configuration. This change was quite opaque as the majority of CI configuration for Cockroach is stored as shell scripts within its repo. This commit follows that pattern by explicitly setting `SKIP_LABEL_TEST_FAILURE` in the script that runs `TestComposeCompare`. Epic: None Release note: None Co-authored-by: Radu Berinde <[email protected]> Co-authored-by: gtr <[email protected]> Co-authored-by: Liam Gillies <[email protected]> Co-authored-by: rharding6373 <[email protected]> Co-authored-by: Rail Aliiev <[email protected]> Co-authored-by: Jayant Shrivastava <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Chris Seto <[email protected]>
blathers-crl bot
pushed a commit
that referenced
this issue
Nov 14, 2023
In TestLargeDynamicRows we test that 4 rows of data can fit in a batch size of at least 4 rows given default memory sizes. However, when we set the batch row size to the minimum value of 4, the test hook that counts batches counts an extra empty batch. This PR changes adjusts the minimum row size to 5 for the purposes of this test. Epic: None Fixes: #109134 Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
pkg/sql/copy/copy_test.TestLargeDynamicRows failed with artifacts on release-23.1 @ ccd59288fdb10e2f2c1e562b2699484ace41148b:
Parameters:
TAGS=bazel,gssHelp
See also: How To Investigate a Go Test Failure (internal)
This test on roachdash | Improve this report!
Jira issue: CRDB-30796
The text was updated successfully, but these errors were encountered: