Marginalia with request_id and job_id bloats PostgreSQLAdapter::StatementPool

[keithpitt]

Hey! I just discovered that if you include the http request_id or the ActiveJob job id inside the marginalia annotation, it will bloat the the PostgreSQLAdapter::StatementPool cache.

Each unique SQL query will create a new prepared_statement entry, and because of the unique annotation, it won't get re-used, so it will just sit around in memory doing nothing. Here is the line inside Rails that generates the prepared statement:

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L644

Here is a screenshot of me listing all the entries in the statement pool, and you'll be able to see a query that could have been re-used, but wasn't because of the unique annotation.

So each unique combination of the annotation will result in a new prepared statement entry. I'm not sure how to fix this, or even if there is a possible fix? Maybe use of marginalia means that you can't use the prepared statement cache?

[ACPK]

Is this still an issue?

[keithpitt]

@ACPK we actually turned off prepared statements in our app, so I'm not entirely sure!

[PikachuEXE]

Should comment be ignored when generating prepared_statement entries?

[sj26]

I've added a warning to the README:

https://github.com/basecamp/marginalia/blob/master/README.md#prepared-statements

Prepared statements can still be useful when using low cardinality components, so I don't think should be automatically disabled. But they're a pretty spikey caveat.

[sj26]

It looks like #54 snuck in a default prepare: false option to queries, too, which might help:

marginalia/lib/marginalia.rb

Lines 60 to 63 in 89d1a1e

	def exec_query_with_marginalia(sql, name = 'SQL', binds = [], options = {})
	options[:prepare] ||= false
	exec_query_without_marginalia(annotate_sql(sql), name, binds, options)
	end