-
Notifications
You must be signed in to change notification settings - Fork 40
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
update to CockroachDB v22.2.19 #5442
Conversation
tools/ci_download_cockroachdb
Outdated
darwin*) | ||
CIDL_BUILD="darwin-10.9-amd64" | ||
if [[ "$2" = "aarch64" ]]; then | ||
CIDL_BUILD="darwin-11.0-arm64" | ||
CIDL_SHA256="$CIDL_SHA256_DARWIN_ARM64" | ||
else | ||
CIDL_BUILD="darwin-10.9-amd64" | ||
CIDL_SHA256="$CIDL_SHA256_DARWIN_AMD64" | ||
fi | ||
CIDL_SUFFIX="tgz" | ||
CIDL_SHA256="$CIDL_SHA256_DARWIN" | ||
CIDL_URL_BASE="$CIDL_URL_COCKROACH" | ||
CIDL_ASSEMBLE="do_assemble_official" | ||
;; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, a small gift for @david-crespo
Perhaps put these in an S3 bucket like the clickhouse artefacts? There is already a bucket that could be renamed and used (and possibly some of the older things cleaned out) |
Yeah I can poke around and see if I can find that S3 bucket or one more appropriate in one of the company accounts. |
tools/ci_download_cockroachdb
Outdated
case "$1" in | ||
darwin*) | ||
CIDL_BUILD="darwin-10.9-amd64" | ||
if [[ "$2" = "aarch64" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically should be ==
, although =
works.
On my mac, $HOSTTYPE
is arm64, not aarch64.
pptang:omicron:main% bash
bash-3.2$ echo $OSTYPE
darwin23
bash-3.2$ echo $HOSTTYPE
arm64
bash-3.2$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm
$ bash
bash-5.2$ echo $HOSTTYPE
aarch64
bash-5.2$ echo $OSTYPE
darwin23.2.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be the difference between bash 3.2, which seems to be what's shipped in MacOS Sonoma as /bin/bash
, versus the bash 5.2 that you're using.
We probably need something like
if [[ "$2" =~ ^(aarch64|arm64)$ ]]; then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should work. Looks like we could also use uname -m
, which returns arm64
consistently, but that is more complicated than sticking to $HOSTTYPE
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apple's refusal to ship a GPLv3-licensed Bash strikes again!!
// `version` will either match `NEWLY_INITIALIZED` while | ||
// running tests normally, or `POLICY` while running tests with | ||
// `CRDB_SEED_USE_PREV=yes`. | ||
let version: CockroachDbClusterVersion = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the changes to this test would land okay in R9 ahead of this PR.
blueprint.cockroachdb_setting_preserve_downgrade = | ||
if cockroachdb_settings.preserve_downgrade.is_empty() { | ||
// Set the option to the current policy in both the database and | ||
// the blueprint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, I think this is fine to land in R9 as well. The else branch here should never get hit in normal rack initialization, we only expect this setting to be non-empty when crdb-seed is running in CRDB_SEED_USE_PREV
mode.
d7fc3b0
to
eaf9120
Compare
Closing in favor of #6396 and its follow-up PR. |
Closes #2017.
This is a draft to prevent merging until R9 goes out, but feel free to review.