Upgrade clickhouse to 23.8 #3009
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #3009 +/- ##
=======================================
Coverage 99.01% 99.01%
=======================================
Files 3 3
Lines 203 203
=======================================
Hits 201 201
Misses 2 2 ☔ View full report in Codecov by Sentry. |
|
Seems like I am encountering an issue where clickhouse immediately is booted with v23.8 instead of incrementally upgrading, which does not seem to be happening locally |
hubertdeng123
force-pushed
the
hubertdeng123/upgrade-ch
branch
from
14b777d
to
3b4c94f
Compare
| @@ -20,6 +20,7 @@ source install/check-latest-commit.sh | |||
| source install/check-minimum-requirements.sh | |||
|
|
|||
| # Let's go! Start impacting things. | |||
| source install/upgrade-clickhouse.sh | |||
There was a problem hiding this comment.
The reason this is placed before turn-things-off is because turn things off script will remove the clickhouse image, which we need to verify the version of clickhouse before proceeding
There was a problem hiding this comment.
Let's add this as a comment in the code, since whoever edits this in the future will probably want to know this fact as well.
| @@ -20,10 +20,8 @@ fi | |||
| export DOCKER_ARCH=$(docker info --format '{{.Architecture}}') | |||
| if [[ "$DOCKER_ARCH" = "x86_64" ]]; then | |||
| export DOCKER_PLATFORM="linux/amd64" | |||
| export CLICKHOUSE_IMAGE="altinity/clickhouse-server:21.8.13.1.altinitystable" | |||
There was a problem hiding this comment.
not needed, since more recent clickhouse images are multi platform
| used on ARM to read the environment variable. --> | ||
| <include from_env="MAX_MEMORY_USAGE_RATIO"/> | ||
| </max_server_memory_usage_to_ram_ratio> | ||
| <!-- This include is important! It is required for the version of Clickhouse used on ARM to read the environment variable. --> |
There was a problem hiding this comment.
For some reason, in more recent versions of clickhouse the newlines here are causing parsing issues, so I just removed them
| echo "${_group}Upgrading Clickhouse ..." | ||
|
|
||
| # First check to see if user is upgrading by checking for existing clickhouse volume | ||
| if [[ -n "$(docker volume ls -q --filter name=sentry-clickhouse)" ]]; then |
There was a problem hiding this comment.
Wonder if it's worth adding a test for this? Should (hopefully) be pretty straightforward with the new python-based testing regime?
There was a problem hiding this comment.
Editing the install script is not easily tested using the new python tests. Those are effective after self-hosted sentry is installed and up and running. The new self-hosted Sentry upgrade tests I added sufficiently covers this IMO, it skips over this portion of the code on initial install and hits this on the upgrade
| @@ -20,6 +20,7 @@ source install/check-latest-commit.sh | |||
| source install/check-minimum-requirements.sh | |||
|
|
|||
| # Let's go! Start impacting things. | |||
| source install/upgrade-clickhouse.sh | |||
There was a problem hiding this comment.
Let's add this as a comment in the code, since whoever edits this in the future will probably want to know this fact as well.
This PR includes a script that upgrades our users to CH 23.8, in this order:
21.8 -> 22.8 -> 23.3 -> 23.8