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.

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

SQLite: Don't bill for internal queries. #3605

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

kentonv
Copy link
Member

@kentonv kentonv commented Feb 25, 2025

We bill for:

  • SQL queries issued via the JavaScript SQL API.
  • DO old-style key/value storage operations.

Anything else, we assume is an internal query, and not billed.

@kentonv kentonv requested review from a team as code owners February 25, 2025 21:10
@shrima-cf
Copy link
Contributor

LGTM

@jclee
Copy link
Contributor

jclee commented Feb 26, 2025

Nice... When I was thinking about how to implement something similar, I was considering adding a "reportingId" value to queries, which could then be passed as an additional parameter to addQueryStats()... That might have the advantage of allowing us to separately track both system query overhead (maybe for internal metrics?) and user query overhead. But not sure if we need that, and if not, this approach seems simpler and more performant.

I guess I also don't know enough about the query stats recording to know if running "ROLLBACK" as a system query -- after running some user queries -- results in significant overhead that we'd want included in user metrics. But maybe that's not a big deal, as long as the stats of the initial execution get recorded and are roughly proportional to what the rollback metrics would be.

@kentonv
Copy link
Member Author

kentonv commented Feb 27, 2025

I guess I also don't know enough about the query stats recording to know if running "ROLLBACK" as a system query -- after running some user queries -- results in significant overhead that we'd want included in user metrics.

The overhead of ROLLBACK is pretty minimal, the WAL just gets truncated back to an earlier point. In fact, any writes that end up rolled back never even leave the local machine, so you could argue a rollback should actually roll back the billing counters! But that would be complicated to implement and awkward to explain.

@kentonv kentonv force-pushed the kenton/sqlite-dont-bill-internal-queries branch from c0bcee8 to 5675a1a Compare March 5, 2025 16:07
@kentonv
Copy link
Member Author

kentonv commented Mar 5, 2025

(rebased)

@kentonv kentonv force-pushed the kenton/sqlite-dont-bill-internal-queries branch from 5675a1a to 8f02131 Compare March 5, 2025 16:08
@kentonv
Copy link
Member Author

kentonv commented Mar 5, 2025

(resolved Joe's comment)

@kentonv
Copy link
Member Author

kentonv commented Mar 5, 2025

ok this time the internal build failure looks legit, I guess I will need to look into it.

We bill for:
- SQL queries issued via the JavaScript SQL API.
- DO old-style key/value storage operations.

Anything else, we assume is an internal query, and not billed.
@kentonv kentonv force-pushed the kenton/sqlite-dont-bill-internal-queries branch from 8f02131 to 91d2ef8 Compare March 10, 2025 23:52
@kentonv kentonv merged commit d15fee3 into main Mar 11, 2025
16 of 17 checks passed
@kentonv kentonv deleted the kenton/sqlite-dont-bill-internal-queries branch March 11, 2025 02:23
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.

5 participants