LookupResources2 follow-ups

[vroldanbet]

Follow up to #1905
Closes #1217

There are several optimizations here, mostly around reducing allocations and GC pressure, and addresses the situation with pgx exec mode, making LR2 faster and learner

changes to default_query_exec_mode

This is a long-standing issue with SpiceDB and prepared statements, and the changes here should address support for PG proxies. As part of the profiling and performance testing I did locally, I observed compelling reasons to move to exec mode. We still need to observe the impact in a real cluster since the protocol is now likely chattier (it's "text" based mode instead of "binary") but remain confident the overhead in the protocol is amortized by the removal of a full roundtrip and the computation of a custom query plan.

Moving default_query_exec_mode=exec showed better LR2 throughput and also reduced allocations. I took the changes from 45c300d and adjusted them so the default mode is now exec which disables prepared plans, which have shown to be counterproductive with SpiceDB.

SpiceDB has been ignoring generic plans for a long time now. The hypothesis is that since SpiceDB queries have so much argument variability the generic plans become suboptimal leading to slow queries. Consequently, we switched to force_custom_plan, meaning a new query plan is created for each query. This wasn't the most efficient choice but it was the available workaround at the time as it could be triggered via URI parameters, and then it was made the default behavior in the code.

From the PG docs:

Prepared statements (either explicitly prepared or implicitly generated, for example by PL/pgSQL) can be executed using custom or generic plans. Custom plans are made afresh for each execution using its specific set of parameter values, while generic plans do not rely on the parameter values and can be re-used across executions. Thus, use of a generic plan saves planning time, but if the ideal plan depends strongly on the parameter values then a generic plan may be inefficient. The choice between these options is normally made automatically, but it can be overridden with plan_cache_mode. The allowed values are auto (the default), force_custom_plan and force_generic_plan. This setting is considered when a cached plan is to be executed, not when it is prepared. For more information see PREPARE.

exec query exec mode does not use the binary protocol, so I had to add support for text support for some of the arguments, specifically xid8 and the map[string]any used for the context column which is mapped to JSONB. Without query plans and with the text mode, there is heavy reliance by pgx over reflection and custom interfaces. I made xid8 implement a pgx interface that bypasses reflection, and map[string]any didn't need a decoder since a JSONB decoder for maps already exists, I just had to register the mapping.

I'm debating if this should be the new default or if we should be more conservative about it. The current configuration is very inefficient and I can't think of reasons this wouldn't be universally beneficial.

Reducing allocations when generating queries

An important reduction in allocations (10% reduction!) which in turn also has a dramatic impact in throughput happens in FilterToResourceIDs and FilterWithSubjectsSelectors. Both concatenated strings to create schema filterers used by LR code. This was extremely inefficient, and by switching to a strings.Builder an important reduction in allocations on a LR-based workload is observed

I also reduced the allocations in other parts of the code that showed up in the profiles and weren't as impactful but seemed like low-hanging fruit (~ 1% reduction).

changes to chunk size

We also observed improvements for certain workloads with wide relations when the dispatch chunk size was changed. This introduces a new --dispatch-chunk-size flag that is also used for datastore query filtering. The default value continues to be 100.

[ecordell]

My understanding is that the plan_cache_mode (a server setting that tells pg how to plan) is orthogonal to the QueryExecMode (a client setting that tells pgx how to send queries to pg).

Are we sure that it's okay to remove force_custom_plan? I would think we'd need both, but maybe I am misunderstanding the full implications of exec.

[vroldanbet]

Indeed it is orthogonal. If we disable prepared statements, there is no longer a reason to change the plan cache mode. We also have incentives to get rid of it, as it will make SpiceDB work out of the box with pgbouncer (requires ignoring the flag) and RDS Proxy (downright does not work because it does not have an option to ignore flags).

See #1217 for more context.

[ecordell]

If we disable prepared statements, there is no longer a reason to change the plan cache mode.

I was under the impression we were already not using statement caching, and were using the mode that just uses prepared statements for type information. But it looks like that isn't correct, and I agree with you that if we just don't use prepared statements at all, there's no reason to send force_custom_plan