-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
84364: test: enforce skip.UnderStress[Race] locally r=[rickystewart] a=HonoreDB Fixes #84312 Previously, skip.UnderStress only checked NightlyStress(), which is only set during nightly stress test runs, not by ./dev test --stress or make stress. This lead to developers checking in tests that flake or reliably fail under stress or stressrace. This PR modifies dev and make to set a COCKROACH_STRESS environment variable, and modifies skip to honor both that flag and the one set by nightly stress. A follow-up might be to default to not skipping when a filter matches only one test, as there the intent was almost certainly to actually run it. Release note: None 84532: cli,sdnotify: improve the handling of unix socket directories r=jreut a=knz tldr: This patch makes it easier for users to understand when CockroachDB has a problem using/creating unix sockets, and what they can do to avert the situation. Fixes #76904. Example, before; ``` $ cockroach start-single-node --insecure --background ... listen unixgram /data/home/kena/cockroach/this-is-a-very-long-directory-name-the-point-is-to-be-more-than-one-hundred-and-twenty-three-characters/sdnotify2139543236/notify.sock: bind: invalid argument ``` Example, after: ``` $ cockroach start-single-node --insecure --background ERROR: no suitable directory found for the --background notify socket HINT: Avoid using --background altogether (preferred), or use a shorter directory name for --socket-dir, TMPDIR or the current directory. ``` **Context:** CockroachDB uses unix sockets both to handle `--background` and to create a postgres-compatible SQL client socket via `--socket-dir`. For `--background`, the logic was previously hardwired to always use `os.TempDir()`, i.e. either `$TMPDIR` or `/tmp`. On Unix, socket objects are commonly limited to 128 bytes; that's e.g. 104 path characters including final NUL on BSD, 108 on glibc. When the actual length is larger, creating the socket fails with a confusing error `bind: invalid argument`. **The problem:** Users have reported issues in the following circumstances: - when setting their CWD/$TMPDIR to a long directory name, they expected `--socket-dir` to also influence the socket name used for `--background`. Instead, `--background` failed with a hard-to-understand error. (#76904) - when running tests inside a container/sandbox with a long prefix, they were confused that `--background` / `--socket-dir=.` did not work properly. - they use macOS which has very large directory names in $TMPDIR. To improve user experience, this patch changes the behavior as described in the release notes below below. Release note (cli change): CockroachDB now produces a clearer error when the path specified via `--socket-dir` is too long. Release note (cli change): When the flag `--background` is specified, CockroachDB now makes 3 attempts to find a suitable directory to create the notification socket: the value of `--socket-dir` if specified, the value of `$TMPDIR` (or /tmp if the env var is empty), and the current working directory. If none of these directories has a name short enough, an explanatory error is printed. 84649: cluster-ui: allow dynamic timescale window from key buttons r=maryliag a=xinhaoz fixes #77007 Previously, controlling the time scale window using the arrow key buttons and 'Now' button would disable the dynamically moving window even when the end time = now. This commit makes it so that when the time window is changed to include the current time (i.e. endTime = now), the time window will be changed to dynamic to ensure the window is keep up to date with the current time. This commit also fixes the object construction of the time scale from session storage and corrects the 'Now' button tooltip label from 'past 1 day' to 'Most recent interval'. Release note (bug fix): changing the time window using arrow buttons and 'Now' button will now properly turn the timeframe into a moving window when endTime = now. https://www.loom.com/share/54d65c04580747dc92f7e4976b953be6 84667: ci: only build `roachprod` on Unix r=jlinder a=rickystewart This fails to build on Windows which was breaking CI. Release note: None Co-authored-by: Aaron Zinger <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]> Co-authored-by: Xin Hao Zhang <[email protected]> Co-authored-by: Ricky Stewart <[email protected]>
- Loading branch information
Showing
28 changed files
with
281 additions
and
96 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
#! /usr/bin/env expect -f | ||
# | ||
source [file join [file dirname $argv0] common.tcl] | ||
|
||
set longname "this-is-a-very-long-directory-name-the-point-is-to-be-more-than-one-hundred-and-twenty-three-characters/and-we-also-need-to-break-it-into-two-parts" | ||
|
||
spawn /bin/bash | ||
send "PS1=':''/# '\r" | ||
eexpect ":/# " | ||
|
||
send "mkdir -p $longname\r" | ||
eexpect ":/# " | ||
|
||
start_test "Check that the socket-dir flag checks the length of the directory." | ||
send "$argv start-single-node --insecure --socket-dir=$longname\r" | ||
eexpect "value of --socket-dir is too long" | ||
eexpect "socket directory name must be shorter" | ||
eexpect ":/# " | ||
end_test | ||
|
||
set crdb [file normalize $argv] | ||
send "export BASEDIR=\$PWD\r" | ||
eexpect ":/# " | ||
send "export PREVTMP=\$TMPDIR\r" | ||
eexpect ":/# " | ||
|
||
start_test "Check that --background complains about the directory name if there is no default." | ||
send "cd $longname\r" | ||
eexpect ":/# " | ||
send "export TMPDIR=\$PWD\r" | ||
eexpect ":/# " | ||
send "$crdb start-single-node --insecure --background\r" | ||
eexpect "no suitable directory found for the --background notify socket" | ||
eexpect "use a shorter directory name" | ||
eexpect ":/# " | ||
end_test | ||
|
||
start_test "Check that --background can use --socket-name if specified and set to sane default." | ||
send "$crdb start-single-node --insecure --background --socket-dir=\$BASEDIR --pid-file=\$BASEDIR/server_pid\r" | ||
eexpect ":/# " | ||
# check the server is running. | ||
system "$crdb sql --insecure -e 'select 1'" | ||
stop_server $crdb | ||
end_test | ||
|
||
start_test "Check that --background can use TMPDIR if specified and set to sane default." | ||
# NB: we use a single-command override of TMPDIR (as opposed to using 'export') so that | ||
# the following test below can reuse the value set above. | ||
send "TMPDIR=\$PREVTMP $crdb start-single-node --insecure --background --pid-file=\$BASEDIR/server_pid\r" | ||
eexpect ":/# " | ||
# check the server is running. | ||
system "$crdb sql --insecure -e 'select 1'" | ||
stop_server $crdb | ||
end_test | ||
|
||
start_test "Check that --background can use cwd if TMPDIR is invalid." | ||
# NB: at this point TMPDIR is still long, as per previous test. | ||
send "cd \$BASEDIR\r" | ||
eexpect ":/# " | ||
send "$crdb start-single-node --insecure --background --pid-file=server_pid\r" | ||
eexpect ":/# " | ||
# check the server is running. | ||
system "$crdb sql --insecure -e 'select 1'" | ||
stop_server $crdb | ||
end_test |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.