Skip to content

Mark schema template replacements as Stable to enable Replacer cache#1242

Merged
brandur merged 3 commits intoriverqueue:masterfrom
IgorDeo:fix-sqlctemplate-schema-stable
May 8, 2026
Merged

Mark schema template replacements as Stable to enable Replacer cache#1242
brandur merged 3 commits intoriverqueue:masterfrom
IgorDeo:fix-sqlctemplate-schema-stable

Conversation

@IgorDeo
Copy link
Copy Markdown
Contributor

@IgorDeo IgorDeo commented May 6, 2026

schemaTemplateParam (and the schema replacement in ColumnExists) never set Stable: true, so the sqlctemplate.Replacer cache introduced in #802 is bypassed on every query. This causes regexp.ReplaceAllStringFunc to run on every SQL query, allocating a new string each time.

The schema value comes from the client config and is constant for the lifetime of a River client, so marking it Stable is safe. With this change, the regex runs once per unique SQL string and then serves from cache.

Fixes all three drivers: riverpgxv5, riverdatabasesql, riversqlite.

Fixes #1241

Copy link
Copy Markdown
Contributor

@brandur brandur left a comment

Choose a reason for hiding this comment

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

Thanks Igor! Looks great.

Would you mind adding an entry under the "fixed" section in the CHANGELOG. Maybe just something like:

- Mark schema replacements as `Stable` in sqlc templates, preventing query SQL from having to be reallocated over and over again.. [PR #1242](https://github.com/riverqueue/river/pull/1242).

@IgorDeo IgorDeo requested a review from brandur May 8, 2026 02:47
@IgorDeo
Copy link
Copy Markdown
Contributor Author

IgorDeo commented May 8, 2026

Thanks Igor! Looks great.

Would you mind adding an entry under the "fixed" section in the CHANGELOG. Maybe just something like:

- Mark schema replacements as `Stable` in sqlc templates, preventing query SQL from having to be reallocated over and over again.. [PR #1242](https://github.com/riverqueue/river/pull/1242).

Done -> 6d208db

Copy link
Copy Markdown
Contributor

@brandur brandur left a comment

Choose a reason for hiding this comment

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

Great. Thanks again!

@IgorDeo
Copy link
Copy Markdown
Contributor Author

IgorDeo commented May 8, 2026

I'm trying to run the tests locally to debug these failures. I see that one of them passed on the third try

@brandur
Copy link
Copy Markdown
Contributor

brandur commented May 8, 2026

@IgorDeo Actually, this looks like it revealed an sqlctemplate bug by accident. I'll take the fix.

brandur added a commit that referenced this pull request May 8, 2026
Related to #1242. Activating caching everywhere revealed a bug in that
we were adding named args using `maputil.Values`. The problem with this
is that `maputil.Values` returns values in an arbitrary order, resulting
in all kinds of odd things potentially happening during a query as args
are switched around.

Here, fix the problem by appending args in the same order as incoming
map keys.
@brandur
Copy link
Copy Markdown
Contributor

brandur commented May 8, 2026

Opened #1243.

brandur added a commit that referenced this pull request May 8, 2026
Related to #1242. Activating caching everywhere revealed a bug in that
we were adding named args using `maputil.Values`. The problem with this
is that `maputil.Values` returns values in an arbitrary order, resulting
in all kinds of odd things potentially happening during a query as args
are switched around.

Here, fix the problem by appending args in the same order as incoming
map keys.
@brandur
Copy link
Copy Markdown
Contributor

brandur commented May 8, 2026

@IgorDeo Hey, just brought in #1243. Would you mind pulling master and rebasing to see if that fixes it?

IgorDeo and others added 3 commits May 8, 2026 12:19
schemaTemplateParam (and the schema replacement in ColumnExists) never
set Stable: true, so the sqlctemplate.Replacer cache introduced in riverqueue#802
is bypassed on every query. This causes regexp.ReplaceAllStringFunc to
run on every SQL query, allocating a new string each time.

Under production throughput this creates enough allocation pressure to
cause visible memory growth — we observed +29MB in 16 minutes on a
worker running river.Start() with continuous job polling.

The schema value comes from the client config and is constant for the
lifetime of a River client, so marking it Stable is safe. With this
change, the regex runs once per unique SQL string and then serves from
cache.

Fixes all three drivers: riverpgxv5, riverdatabasesql, riversqlite.

Fixes riverqueue#1241
@IgorDeo IgorDeo force-pushed the fix-sqlctemplate-schema-stable branch from 56118b5 to 41e5084 Compare May 8, 2026 15:19
@IgorDeo IgorDeo requested a review from brandur May 8, 2026 15:22
@brandur brandur merged commit 87c0c50 into riverqueue:master May 8, 2026
12 checks passed
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.

schemaTemplateParam should mark schema replacement as Stable: true to enable Replacer cache

2 participants