Skip to content

fix clickhouse flaky tests#1196

Merged
BilalG1 merged 3 commits intodevfrom
clickhouse-sync-fixes
Feb 13, 2026
Merged

fix clickhouse flaky tests#1196
BilalG1 merged 3 commits intodevfrom
clickhouse-sync-fixes

Conversation

@BilalG1
Copy link
Copy Markdown
Collaborator

@BilalG1 BilalG1 commented Feb 13, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Improved system resilience: database synchronization now continues seamlessly if one data source becomes temporarily unavailable.
  • Chores

    • Optimized internal database schema and query performance.

@vercel
Copy link
Copy Markdown

vercel bot commented Feb 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
stack-backend Ready Ready Preview, Comment Feb 13, 2026 9:13pm
stack-dashboard Ready Ready Preview, Comment Feb 13, 2026 9:13pm
stack-demo Ready Ready Preview, Comment Feb 13, 2026 9:13pm
stack-docs Ready Ready Preview, Comment Feb 13, 2026 9:13pm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 13, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

This pull request updates ClickHouse migration scripts and related backend services to normalize sign-up-rule-trigger event data, converts three user table metadata columns from JSON to String types, optimizes metrics query construction to handle large user ID lists via inline array literals, and adds end-to-end tests validating ClickHouse resilience when Postgres is unavailable.

Changes

Cohort / File(s) Summary
ClickHouse Schema & Migration
apps/backend/scripts/clickhouse-migrations.ts
Added SIGN_UP_RULE_TRIGGER_EVENT_ROW_FORMAT_MUTATION_SQL constant for normalizing sign-up-rule-trigger event data (camelCase to snake_case conversion). Updated USERS_TABLE_BASE_SQL to change client_metadata, client_read_only_metadata, and server_metadata columns from JSON to String type.
Metrics Query Optimization
apps/backend/src/app/api/latest/internal/metrics/route.tsx
Replaced structured parameter has({userIds:Array(String)}, ...) with inline ClickHouse array literal (userIdsArrayLiteral) to avoid URL parameter size limits when filtering by multiple user IDs. Removed userIds from query parameters as IDs are now passed inline in the query body.
External DB Sync Serialization
apps/backend/src/lib/external-db-sync.ts
Added JSON string serialization for client_metadata, client_read_only_metadata, and server_metadata fields in ClickHouse payload rows returned by pushRowsToClickhouse.
Resilience Test Coverage
apps/e2e/tests/backend/endpoints/api/v1/external-db-sync-advanced.test.ts
Added runQueryForCurrentProject and waitForClickhouseUser helper functions. Introduced new test "Cross-DB resilience: postgres down does not block ClickHouse sync" verifying that ClickHouse synchronization continues independently when Postgres connection is unavailable. Updated imports to include Project from backend-helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 Hoppy migrations, schemas refined,
Metadata strings dance in perfect alignment,
Array literals leap past URL size confines,
While resilient tests ensure no line's maligned,
ClickHouse hops freely when Postgres reclines! 🏃‍♂️✨

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch clickhouse-sync-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@BilalG1 BilalG1 marked this pull request as ready for review February 13, 2026 21:04
@BilalG1 BilalG1 merged commit 5b149be into dev Feb 13, 2026
1 check passed
@BilalG1 BilalG1 deleted the clickhouse-sync-fixes branch February 13, 2026 21:05
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 13, 2026

Greptile Overview

Greptile Summary

Fixed flaky ClickHouse tests by addressing two distinct issues: schema type mismatches and HTTP parameter size limits. Changed client_metadata, client_read_only_metadata, and server_metadata fields from JSON to String type in ClickHouse schema to prevent type-related failures, with corresponding JSON.stringify() calls during data insertion. Resolved HTTP form field size limit errors when querying many user IDs by embedding the array literal inline in the query body instead of passing via URL parameters. Added migration for normalizing legacy sign-up-rule-trigger event data and a test demonstrating cross-database resilience.

Confidence Score: 5/5

  • Safe to merge with minimal risk
  • Changes are well-targeted fixes for flaky tests with clear root causes. Schema migration properly handles existing data, JSON stringification is applied consistently, and the query parameter fix uses proper SQL escaping. Test additions validate cross-DB resilience.
  • No files require special attention

Important Files Changed

Filename Overview
apps/backend/scripts/clickhouse-migrations.ts Added migration for sign-up-rule-trigger events and changed metadata fields from JSON to String type
apps/backend/src/app/api/latest/internal/metrics/route.tsx Fixed HTTP form field size limit by embedding user IDs inline in query instead of URL params
apps/backend/src/lib/external-db-sync.ts Stringify metadata JSON objects before inserting to ClickHouse to match new String schema
apps/e2e/tests/backend/endpoints/api/v1/external-db-sync-advanced.test.ts Added test for cross-database resilience proving ClickHouse and Postgres syncs are independent

Last reviewed commit: d8eca01

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant