Skip to content

Conversation

@Fenoman
Copy link
Contributor

@Fenoman Fenoman commented May 7, 2025

Skips client-side DISCARD ALL statements instead of forwarding them to the backend (preventing it from reaching PostgreSQL).

Closes #788

@rkhapov
Copy link
Collaborator

rkhapov commented May 9, 2025

Lets add the tests for this functionality, like the tests in docker/ folder

@Fenoman Fenoman force-pushed the ignore_discardall branch from 14421cc to 9b198ec Compare May 28, 2025 08:14
@Fenoman Fenoman force-pushed the ignore_discardall branch 2 times, most recently from 24bed6f to 67c4414 Compare August 4, 2025 02:12
Introduces a new pool configuration option `pool_ignore_discardall` that allows
Odyssey to intercept and handle DISCARD ALL commands without forwarding them
to PostgreSQL.

When enabled, the pooler responds with a successful completion message while preserving packet order through sync points. This feature prevents unnecessary connection resets caused by DISCARD ALL commands from certain clients.

Changes:
- Add pool_ignore_discardall configuration parameter
- Implement DISCARD ALL interception in frontend.c with efficient string matching
- Preserve packet ordering using sync point mechanism
- Add configuration parsing and logging support
@Fenoman Fenoman force-pushed the ignore_discardall branch from 67c4414 to fbe4313 Compare August 5, 2025 00:53
bool match = true;

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

Choose a reason for hiding this comment

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

why not use strncasecmp ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are absolutely right—strncasecmp is an excellent and readable function. However, in this particular case, the choice to use a manual implementation for string comparison is driven, first and foremost, by considerations of extreme performance.

This code is located in the project's "hot path"—it executes for every Simple Query command that passes through the connection pool and meets the length requirement. In our specific use case, as I've mentioned before, we have quite a few microservices that explicitly and intentionally issue DISCARD ALL commands, and disabling this behavior is challenging. My reasoning was that even minimal overheads accumulate and can impact the overall request processing latency. A direct implementation of the comparison loop allows the compiler to apply aggressive optimizations (like loop unrolling) and generate highly efficient machine code, free from side effects and dependencies. Beyond pure performance, this approach has other significant advantages: portability (though perhaps less critical in our context), and predictability with simplicity (the implementation is not dependent on system locales and doesn't require including <strings.h>).

@rkhapov
Copy link
Collaborator

rkhapov commented Aug 5, 2025

you need to run make format

consider creating test for this behaviour, see https://raw.githubusercontent.com/yandex/odyssey/master/docs/development/testing.md

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.

discard "DISCARD ALL" query

2 participants