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

Jump to bottom

chore: Add ClickHouse version 22.3 to CI and set 21.11.11.1 to be default except for ARM64 compose #9743

Merged
merged 7 commits into from
May 16, 2022
Merged

chore: Add ClickHouse version 22.3 to CI and set 21.11.11.1 to be default except for ARM64 compose #9743

merged 7 commits into from
May 16, 2022

Conversation

fuziontech
Copy link
Member

@fuziontech fuziontech commented May 11, 2022 •
edited
Loading

Problem

We'd like to migrate to 22.3 sooner than later. We also want to upgrade the cluster to 21.11.11.1 for the short term before 22.3

Changes

  • Add 22.3 to the CI matrix.
  • Set 21.11.11.1 as the default
  • Set 22.3 as default for Arm64 because it's official build (and all tests pass)

How did you test this code?

CI and locally

@fuziontech fuziontech changed the title Remove ClickHouse version 21.6 and add 22.3 for testing and docker-compose (21.6 is not in clickhouse dockerhub) chore - Remove ClickHouse version 21.6 and add 22.3 for testing and docker-compose (21.6 is not in clickhouse dockerhub) May 11, 2022
@fuziontech
chore: Remove ClickHouse version 21.6 and add 22.3 for testing and do…
1194c2b 
…cker-compose (21.6 is not in clickhouse dockerhub)
@fuziontech fuziontech changed the title chore - Remove ClickHouse version 21.6 and add 22.3 for testing and docker-compose (21.6 is not in clickhouse dockerhub) chore: Remove ClickHouse version 21.6 and add 22.3 for testing and docker-compose (21.6 is not in clickhouse dockerhub) May 11, 2022
@fuziontech fuziontech mentioned this pull request May 11, 2022
Merged
6 tasks
rcmarron
rcmarron reviewed May 11, 2022
View reviewed changes
Copy link
Contributor

@rcmarron rcmarron left a comment

Choose a reason for hiding this comment

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

This looks good to me. I ran it locally, and everything checks out, but I'm a bit out of my wheelhouse, so I'd definitely defer to @macobo for the final ✅

@macobo
Copy link
Contributor

macobo commented May 12, 2022 •
edited
Loading

I think looping in infra (@hazzadous or @guidoiaquinti) makes sense here.

I think the self-hosted deployment part here needs work. My worry is that we should still test w/ 21.6 since real setups might be running it a while - we don't force people to update the chart when updating posthog and hence they won't be on the new version as part of the upgrade.

Also we should update the chart repo - the current values.yaml lists yandex repo This exists PostHog/charts-clickhouse#381

@hazzadous
Copy link
Contributor

hazzadous commented May 12, 2022 •
edited
Loading

I think the self-hosted deployment part here needs work. My worry is that we should still test w/ 21.6 since real setups might be running it a while - we don't force people to update the chart when updating posthog and hence they won't be on the new version as part of the upgrade.

We should have a grace period to give people time to upgrade their chart. After this we can drop 21.6 support.

@fuziontech we can instead change clickhouse-server-image-version: 21.6.5 -> clickhouse-server-image: yandex/clickhouse-server:21.6.5 etc and keep only 21.6.5 and 22.3

@guidoiaquinti
Copy link
Contributor

+1 on the above. Here is my proposal:

  1. we complete the 21.11 upgrade on PostHog Cloud
  2. we upgrade self-hosted to 21.11. At this point, the 21.11 will be completed and it will be part of the next release.
  3. we make sure 22.3 works on our codebase (I see CI is unhappy but it could be unrelated)
  4. we add 22.3 to the mix of versions we support in CI (21.6, 21.11, 22.3)
  5. we upgrade the docker compose files to use 22.3
  6. the following release we drop the support for 21.6 (removing the support in CI as well)

@fuziontech
Default to 21.11.11.1 and support multiple dockerhub repositories
2b9d84a
@fuziontech
prettier .github/workflows/ci-backend.yml
60baf71
@fuziontech
Merge branch 'master' into clickhouse_LTS
8423d59 
* master:
  fix(cohorts): num comparison (#9764)
  fix(trends): Use 0 as fallback data point value (#9766)
  fix: Flexing of PropertyFilterButton (#9761)
  chore(funnels): Add `lemon-funnel-viz` to `PERSISTED_FEATURE_FLAGS` (#9757)
  feat(insights): New and improved ActionFilter (#9741)
  fix: make the kafka inspector work again after kea-forms update (#9758)
  fix: repace underscores with spaces in the cloud announcement (#9751)
  chore(contributors): 🤖 - Add utkuzih as a contributor 🎉 (#9748)
  fix: cohort person property value not populated (#9745)
  fix: make cohort name field error consistent (#9744)
  fix(cohort): don't let time_value be less than 0 (#9742)
@fuziontech fuziontech changed the title chore: Remove ClickHouse version 21.6 and add 22.3 for testing and docker-compose (21.6 is not in clickhouse dockerhub) chore: Add 22.3 to CI and set 21.11.11.1 May 13, 2022
@fuziontech fuziontech changed the title chore: Add 22.3 to CI and set 21.11.11.1 chore: Add ClickHouse version 22.3 to CI and set 21.11.11.1 to be default except for ARM64 compose May 13, 2022
@fuziontech
Merge branch 'master' into clickhouse_LTS
7478bbc 
* master:
  fix: exclusion steps cannot be selected (#9762)
  feat(lemon-button): Support `status` for `primary` buttons (#9782)
  fix: healthcheck for kafka on plugin server (#9771)
  fix(billing): Update billing success message (#9739)
  fix(plugin-server): Set transpileOnly when importing piscina code in tests (#9777)
  fix(plugin-server): Remove unused kafka reset test code (#9779)
  fix: set kafka_skip_broken_messages on dead letter queue table (#9754)
  fix(plugin-server): remove dead code from worker.test.ts (#9776)
  refactor(plugin-server): Run ingestion only on worker threads (#9738)
  fix: Plugin-server tests with kafka need to have consumer stopped (#9774)
  chore(deps): Update posthog-js to 1.21.1 (#9773)
  chore(dep): upgrading rr-web (#9772)
  fix: ouroboros inputs (#9769)
@fuziontech fuziontech merged commit 49c56fa into master May 16, 2022
@fuziontech fuziontech deleted the clickhouse_LTS branch May 16, 2022 21:00
neilkakkar added a commit that referenced this pull request May 17, 2022
@neilkakkar
fix CH server image version #9743
f17b33b
alexkim205 pushed a commit that referenced this pull request May 23, 2022
@fuziontech @alexkim205
chore: Add ClickHouse version 22.3 to CI and set 21.11.11.1 to be def…
449e749 
…ault except for ARM64 compose (#9743)

* chore: Remove ClickHouse version 21.6 and add 22.3 for testing and docker-compose (21.6 is not in clickhouse dockerhub)

* update service requirements

* update arm64 compose as well

* Default to 21.11.11.1 and support multiple dockerhub repositories

* prettier .github/workflows/ci-backend.yml
EDsCODE added a commit that referenced this pull request May 23, 2022
@neilkakkar @EDsCODE @github-actions
feat(retention): Enable person on events querying for retention (#9858)
5e09eba 
* add constance setting

* add minimum viable code

* update journeys_for

* working test case

* chore: remove eventserializer in unnecessary places

* change execution

* change imports

* fix typing and tests

* change format

* adjust

* reset

* split group path in trend breakdown

* fix: typing

* fix test

* fix test, rename

* automatically add persons to events in tests on flush

* test CI with person-on-events

* wip

* fix

* fix ci

* add comma

* reduce test explosion

* fix CH server image version #9743

* update conditional

* split person on events

* split person on events

* split properly

* fix test deps

* add persons on events for tests

* rm eventserializer changes

* upd

* fix tests

* streamline person on event tests

* remove cache dependence, even after dematerialization deletion

* disregard snapshot tests

* no casting for matrix variables

* fix actions problems

* try to limit blast radius

* Update snapshots

* why are these tests passing?

* stoopid

* fix trend tests

* fix more trend tests, add test for old query regression

* Update snapshots

* optimise persons

* feat(retention): Enable person on events querying for retention

* update snaps

* Update snapshots

* Update snapshots

* fix tests

Co-authored-by: eric <eeoneric@gmail.com>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rcmarron rcmarron rcmarron left review comments

@macobo macobo Awaiting requested review from macobo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fuziontech @macobo @hazzadous @guidoiaquinti @rcmarron