Add a flag to start writing usage to ClickHouse #10806
Open
+332
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bduffany
force-pushed
the
usage-olap2
branch
5 times, most recently
from
fa2678b to
01eab20
Compare
vanja-p
reviewed
Dec 2, 2025
| "test.recycle-runner": "true", | ||
| "test.runner-recycling-key": "clickhouse:25.3", | ||
| }, | ||
| tags = ["docker"], |
Member
Author
There was a problem hiding this comment.
It indicates that the test requires docker to be available in the execution environment. The effect is that the test is excluded when running bazel test //... locally.
enterprise/server/usage/usage.go
Outdated
| var ( | ||
| region = flag.String("app.region", "", "The region in which the app is running.") | ||
| region = flag.String("app.region", "", "The region in which the app is running.") | ||
| writeToOLAP = flag.Bool("app.write_usage_to_olap_db", false, "If true, write usage data to OLAP DB.") |
Contributor
There was a problem hiding this comment.
If true, write usage data to OLAP DB.
Maybe specify that this is in addition to the primary write.
enterprise/server/usage/usage.go
Outdated
| return err | ||
| } | ||
| // Update counts in the DB | ||
| // TODO: for ClickHouse, we should flush everything in a single |
Contributor
There was a problem hiding this comment.
I think we have to do this now since the current approach doesn't play nicely with async inserts.
- Only one app is flushing at a time.
- Because we use
wait_for_async_insert=trueandasync_insert_busy_timeout_max_ms=200, each write can take up to 200ms. - We don't start the next write until the first is completed.
This means that we don't do any batching, and the progress will be very slow.
vanja-p
approved these changes
Dec 3, 2025
|
|
||
| // Flush a accumulated OLAP rows. | ||
| if len(olapRows) > 0 { | ||
| if err := ut.env.GetOLAPDBHandle().FlushUsages(ctx, olapRows); err != nil { |
Contributor
There was a problem hiding this comment.
what do you think about adding a check that GetOLAPDBHandle() isn't nil, so that we can return an error instead of panicking?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes it so we can start writing usage data to ClickHouse (behind a flag) using the same
UsageTrackerinterface, redis buffering logic, and DB flushing logic as the existing MySQL-based usage tracker.To be able to reuse the same tracking interface, we transparently convert the MySQL usage rows to the new ClickHouse format. The benefit of starting out this way is that it allows us to start populating the DB so that we can start sorting out any data discrepancies sooner rather than later.
Once I've sorted out any data discrepancies, I plan on adding a first-class interface + dedicated redis entries for OLAP DB usage tracking, rather than reusing the primary DB structs and redis storage. At that point, we'll be able to start attaching arbitrary labels to usage rows, giving us the full benefit of ClickHouse (on top of just the performance benefits).