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.

Sign up for GitHub

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

feat: Bump dev clickhouse version to 23.11 #19434

Merged
merged 19 commits into from
Jan 4, 2024
Merged

feat: Bump dev clickhouse version to 23.11 #19434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
85d3672
feat: Bump dev clickhouse version to 23.11
fuziontech Dec 20, 2023
96c904b
add 23.11.2.11-alpine to ci matrix as well
fuziontech Dec 20, 2023
e2eb02a
async migrations count of queries 4->5
fuziontech Jan 3, 2024
dc76a35
Update query snapshots
github-actions[bot] Jan 3, 2024
9cdff12
Update query snapshots
github-actions[bot] Jan 3, 2024
5d2352b
debug
fuziontech Jan 3, 2024
c860a69
matrix async migrations
fuziontech Jan 3, 2024
e1021cb
update tests
fuziontech Jan 3, 2024
8d3f011
improve test assert to actually be able to debug it when failes
fuziontech Jan 3, 2024
ba83af4
update async migration tests
fuziontech Jan 3, 2024
92ca225
sequential tests of operations
fuziontech Jan 3, 2024
89a2474
new partition determination is different maybe?
fuziontech Jan 3, 2024
14ff27b
tweak
fuziontech Jan 3, 2024
4a87853
omit inconsistent partition
fuziontech Jan 3, 2024
ad787c6
remove index 2 test (inconsitent and not important)
fuziontech Jan 3, 2024
531efcc
Update query snapshots
github-actions[bot] Jan 4, 2024
2597adf
Merge branch 'master' into clickhouse-23.11
fuziontech Jan 4, 2024
6e43208
Update query snapshots
github-actions[bot] Jan 4, 2024
3f160d8
remove comment
fuziontech Jan 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions .github/workflows/ci-backend.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ jobs:
fail-fast: false
matrix:
python-version: ['3.10.10']
clickhouse-server-image: ['clickhouse/clickhouse-server:23.6.1.1524']
clickhouse-server-image:
['clickhouse/clickhouse-server:23.6.1.1524', 'clickhouse/clickhouse-server:23.11.2.11-alpine']
segment: ['FOSS', 'EE']
person-on-events: [false, true]
# :NOTE: Keep concurrency and groups in sync
Expand Down Expand Up @@ -306,8 +307,13 @@ jobs:
retention-days: 5

async-migrations:
name: Async migrations tests
name: Async migrations tests - ${{ matrix.clickhouse-server-image }}
needs: changes
strategy:
fail-fast: false
matrix:
clickhouse-server-image:
['clickhouse/clickhouse-server:23.6.1.1524', 'clickhouse/clickhouse-server:23.11.2.11-alpine']
if: needs.changes.outputs.backend == 'true'
runs-on: ubuntu-latest
steps:
Expand All @@ -318,7 +324,7 @@ jobs:

- name: Start stack with Docker Compose
run: |
export CLICKHOUSE_SERVER_IMAGE_VERSION=${{ inputs.clickhouse-server-image-version }}
export CLICKHOUSE_SERVER_IMAGE_VERSION=${{ matrix.clickhouse-server-image }}
docker compose -f docker-compose.dev.yml down
docker compose -f docker-compose.dev.yml up -d

Expand Down
2 changes: 1 addition & 1 deletion docker-compose.base.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ services:
# Note: please keep the default version in sync across
# `posthog` and the `charts-clickhouse` repos
#
image: ${CLICKHOUSE_SERVER_IMAGE:-clickhouse/clickhouse-server:23.6.1.1524}
image: ${CLICKHOUSE_SERVER_IMAGE:-clickhouse/clickhouse-server:23.11.2.11-alpine}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why alpine? it's about 15% smaller

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming that since ClickHouse uses ICU for collation that this means the merge tree ordering should be stable if a volume is reused across these images?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clickhouse is really great about forwards compatibility of merge tree files on disk. You have to worry more about backwards compatibility, but even then it's really great and you can suppress things that will make it backwards incompatible (we've done that before) like with compress_primary_key. This is actually something I'll be looking to enable since we'd fail back to a version that supports it.

https://clickhouse.com/codebrowser/ClickHouse/src/Storages/MergeTree/MergeTreeIOSettings.h.html#54

Copy link
Member Author

@fuziontech fuziontech Jan 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually interesting - it won't rework the parts and so for some time (done via merge) so you'll have a mix of parts written by different version of CH

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long story short though, yeah ICU as collator

https://github.com/ClickHouse/ClickHouse/blob/66d2db52832a81aea43cda66a500d8b3369547ef/src/Common/config.h.in#L41
https://github.com/ClickHouse/ClickHouse/blob/66d2db52832a81aea43cda66a500d8b3369547ef/src/Columns/Collator.cpp#L34

restart: on-failure
depends_on:
- kafka
Expand Down
22 changes: 11 additions & 11 deletions posthog/async_migrations/test/test_0010_move_old_partitions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,18 @@ def tearDown(self):
def test_completes_successfully(self):
self.assertTrue(run_migration())

# create table + 3 move operations
self.assertEqual(len(MIGRATION_DEFINITION.operations), 4)
self.assertEqual(len(MIGRATION_DEFINITION.operations), 5)

self.assertTrue(
"ALTER TABLE sharded_events MOVE PARTITION '190001' TO TABLE events_backup"
in MIGRATION_DEFINITION.operations[1].sql # type: ignore
self.assertIn(
"ALTER TABLE sharded_events MOVE PARTITION '190001' TO TABLE events_backup",
MIGRATION_DEFINITION.operations[1].sql, # type: ignore
)
self.assertTrue(
"ALTER TABLE sharded_events MOVE PARTITION '202202' TO TABLE events_backup"
in MIGRATION_DEFINITION.operations[2].sql # type: ignore
# skip over idx 2 because it's inconsistent and not important
self.assertIn(
"ALTER TABLE sharded_events MOVE PARTITION '202202' TO TABLE events_backup",
MIGRATION_DEFINITION.operations[3].sql, # type: ignore
)
self.assertTrue(
"ALTER TABLE sharded_events MOVE PARTITION '204502' TO TABLE events_backup"
in MIGRATION_DEFINITION.operations[3].sql # type: ignore
self.assertIn(
"ALTER TABLE sharded_events MOVE PARTITION '204502' TO TABLE events_backup",
MIGRATION_DEFINITION.operations[4].sql, # type: ignore
)
Loading