discard "DISCARD ALL" query

[Fenoman]

Would it be possible to add a new configuration setting (e.g., pool_discard_discardall)?

When this setting is enabled, Odyssey would intercept the DISCARD ALL command sent directly by the client connection driver and effectively ignore it (preventing it from reaching PostgreSQL).

This feature would be particularly useful in environments where:

1. A significant amount of business logic is implemented within the database itself.
2. Temporary tables are used very intensively.
3. To combat system catalog bloat, it's desirable to reuse temporary tables across pooled sessions instead of dropping them (which DISCARD ALL does) and then recreating them later.
4. Unfortunately, the specific client driver being used does not provide an option to disable the automatic sending of the DISCARD ALL command (e.g., when a connection is returned to the pool).

By intercepting and ignoring DISCARD ALL at the pooler level, we could achieve the desired temporary table reuse even when the client driver enforces sending this command.

[reshke]

So this is meant for session pooling, since in tx pooling your temp tables just goes away to another backend between transactions?

[Fenoman]

Actually, this setting is needed for both transaction and session pooling modes! We are indeed using transaction pooling, as counter-intuitive as that might sound at first glance.

Let me explain the details for better context:

We have disabled the default pool_discard option and are using a custom query via the pool_discard_query setting (the specifics of the query aren't relevant here). We run numerous microservices that connect to the database through Odyssey. While we've disabled client-side pooling in some services, it's not feasible everywhere, making Odyssey's transaction pooling mode the most suitable overall approach for us currently.

As mentioned in the issue description, we have a significant amount of complex business logic implemented in PL/pgSQL, which heavily relies on temporary tables. Many services execute queries that call database functions. These functions often create, populate, and then process temporary tables (e.g., for generating reports or performing data operations).

This constant cycle of DROP TEMP TABLE / CREATE TEMP TABLE leads to severe system catalog bloat in a very short amount of time. Furthermore, it triggers frequent cache invalidation waves across all backends, as the catalog information needs to be updated everywhere.

To combat this, we've implemented the following strategy:

1. A client connects via Odyssey and calls a database function.
2. Odyssey assigns a physical backend connection to the client. This backend might already contain some or all of the necessary temporary tables, created by a previous client session(transaction) that used the same backend connection.
3. Inside the PL/pgSQL function, instead of blindly issuing DROP/CREATE, we check if the required temporary table exists:
   • If it does not exist, we CREATE it.
   • If it does exist, we simply clear its data using SELECT fasttruncate('temp_table_name'); (from the fasttrun extension). We use this instead of a standard TRUNCATE because regular TRUNCATE can also contribute significantly to catalog bloat in high-frequency scenarios.
4. If a subsequent query using the same backend connection is purely read-only, the presence of these pre-existing temporary tables doesn't cause any issues.
5. We have configured relatively high pool_ttl and server_lifetime values. This ensures that physical backend connections stay alive for as long as possible, allowing them to "accumulate" the temporary tables over time.
6. As the server runs, temporary tables accumulate within the persistent backend connections. The probability that a function call finds its required temporary table already existing increases significantly.

This strategy has substantially reduced our system catalog bloat by promoting the reuse of temporary tables (we just clear the data). Of course, if the PostgreSQL server restarts, or if a client does manage to send a DISCARD ALL (which is the core point of this issue), the "accumulation" process for temporary tables restarts in the affected backends, and catalog bloat begins to grow again – albeit much more slowly than before implementing this strategy.

The main problem we're facing now is that some client applications or drivers cannot be configured to prevent sending DISCARD ALL when they release a connection back to the pool (even Odyssey's pool). These clients effectively "spoil the strategy" by clearing out all the accumulated temporary tables in the backend connections they use, forcing the slow catalog growth to continue.

So, to directly answer your question:
Yes, we rely on the possibility that "temp tables just go away to another backend between transactions".

In transaction pooling, if the same physical backend connection is reused for a subsequent transaction (from the same or different client), we want the temporary tables created earlier (within that backend) to persist. The unavoidable DISCARD ALL from certain clients breaks this persistence.

That's why intercepting and ignoring DISCARD ALL at the pooler level is crucial for us, even in transaction mode.

[reshke]

Alright, this use case is legitimate.
The one thing I'm thinking about is whether we should indeed discard this query, or rewrite it to pool_discard_query

[Fenoman]

Thank you for considering the feature.

whether we should indeed discard this query, or rewrite it to pool_discard_query

I don’t think rewriting into pool_discard_query is necessary:

• DISCARD ALL, like the other DISCARD sub-commands, is a utility statement.
  PostgreSQL refuses to run it inside an explicit transaction block and returns
  ERROR: DISCARD ALL cannot run inside a transaction block.
  Hence every DISCARD ALL is executed as its own autocommit transaction.
• In transaction pooling a driver-issued DISCARD ALL may hit a random physical
  connection (whichever Odyssey assigns at that moment) and wipe all its session
  state before the usual pool_discard_query is executed on detach.

For that reason simply ignoring the statement is the safest solution.

Actually, I forked the repository over the weekend and, drawing on my C programming expertise and a quick review of the Odyssey source code, implemented a minimal and non-intrusive patch - Fenoman@bc04581.

The idea is to keep the hot path as cheap as possible, the logic lives only in
frontend.c.

Key points:

1. Early size guard

   /* FE Query header: 1 (type) + 4 (length) = 5 bytes */
   const uint32_t MAX_RELEVANT = 5 + 15;          /* 20 bytes total */
           
   if ((uint32_t)size <= MAX_RELEVANT)            /* longer packets → skip */
   {
       ...
   }

   99 % of real traffic is filtered by one comparison and never enters the slower path.

2. Cheap parse for short packets
   Call kiwi_be_read_query() only for packets ≤ 20 B; no allocations.

3. Trimming overhead ≤ 4 bytes

   • Remove leading/trailing whitespace.
   • Drop optional semicolon and terminating \0.

4. Fixed-length, branch-free compare

   • After trimming, payload must be exactly 11 bytes.
   • Case-insensitive match is 11 bytewise operations

    for (int i = 0; i < 11; ++i)
        if ( (q[i] | 32) != (pat[i] | 32) ) { match = false; break; }

5. Fast reply, no backend write

   • On match, build CommandComplete: DISCARD + ReadyForQuery in a pre-allocated machine_msg_t and return OD_SKIP.

Estimated overhead: worst-case ~70 ns; for non-matching packets the cost is ≈ 1 ns (one integer comparison).
Even at 100 k simple queries per second the filter approximately burns < 0.7 % of a single CPU core.

Local tests look good; this week we’ll run it on a staging server and report back.

[rkhapov]

Actually, I forked the repository over the weekend and, drawing on my C programming expertise and a quick review of the Odyssey source code, implemented a minimal and non-intrusive patch - Fenoman@bc04581.

looks great, can you create a pr with this patch?

[Fenoman]

Certainly—that was my plan!
Give me a couple of days to verify the change under "real-world" load. Once I’m sure everything behaves correctly, I’ll open the PR right away.

[reshke]

You cannot write msgs right away here Fenoman@bc04581#diff-d23e9ec6d450d0487f0e95425e6eaea8e76945e520a720a69cf0efd2cc0403dcR1183
because of pipe-lining. You need to wait for all preceding msgs to reach client first. This can be achieved using OD_REQ_SYNC. I did small refactoring in b69c18c to showcase this.

[reshke]

For example, for prepared statement in tx pool support we use OD_REQ_SYNC here

odyssey/sources/frontend.c

Line 1213 in b69c18c

retstatus = OD_REQ_SYNC;

This way we send parse complete msgs only after replying to all prev msgs in pipe

[Fenoman]

Thank you for this clarification. I will carefully review everything shortly!

[Fenoman]

First of all, thank you for the detailed feedback on my previous patch!
I realize now that I’d overlooked an important detail in the sync-point logic—sorry for this 😊.

I have pushed a new revision to my fork Fenoman@d32ab1d. Key changes:

1. Sync-point handling
   OD_REQ_SYNC can now be returned in two distinct situations:

   • the original reserve-prepared-statement path (where server->parse_msg is set), and
   • the new ignore-DISCARD-ALL path (where no parse_msg is generated).

   Because of that I replaced the hard assertion:

   assert(server->parse_msg != NULL);

   with a conditional block:

   if (server->parse_msg) {
       /* fill internals structs in */
       if (od_frontend_deploy_prepared_stmt_msg(...
   }

   This keeps the old behaviour intact and avoids an abort() when parse_msg is legitimately NULL for the DISCARD ALL.

2. Packet ordering
   The virtual CommandComplete + ReadyForQuery message is now stored in server->sync_point_deploy_msg.
   The main loop flushes it right after the sync-point, so pipelining is preserved.

3. Cache invalidation
   server->prep_stmts is only cleared when a real DISCARD … reaches PostgreSQL.
   When we intercept DISCARD ALL, prepared statements remain valid.

Everything builds and the my local tests passes. Please take a look when you have a minute.

[reshke]

2. **Packet ordering**
   The fake _CommandComplete + ReadyForQuery_ message 

I prefer to call them "virtual" akin to what SPQR does

[Fenoman]

Fixed :-)

[Fenoman]

Btw, regarding SPQR—would it be possible to implement both the existing pool_discard_query feature and the new pool_ignore_discardall option there as well?

pool_discard_query is extremely useful in certain cases—for example, cleaning up pgv variables (pg_variables extension), because they can be cleared ONLY by calling SELECT pgv_free();.

We’re planning to experiment with sharding some of our databases, and having these features available in SPQR is essential for us.

[Fenoman]

Hi everyone,

After testing on several stage and then prod servers, I can confirm that everything is working wonderfully!
Therefore, I'm opening a PR for this.