Fix Npgsql connection pool and update configurations#425
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR makes configuration changes across multiple parts of the codebase to optimize deployment and development workflows. However, only the C# configuration change is visible in the provided diffs.
Changes:
- Disables connection state reset in Npgsql connection pool for performance optimization
- (Mentioned but not visible) Updates Python deployment configuration
- (Mentioned but not visible) Reorganizes development tooling configuration
| @@ -154,6 +154,7 @@ private static string ConvertDatabaseUrlToNpgsqlConnectionString(string database | |||
| Host = parsedUrl.Host, | |||
| Port = parsedUrl.Port > 0 ? parsedUrl.Port.Value : 5432, | |||
| Database = Uri.UnescapeDataString(databaseName), | |||
There was a problem hiding this comment.
The NoResetOnClose = true setting has been added without documentation or explanation. This setting prevents Npgsql from issuing DISCARD ALL commands when returning connections to the pool, which can impact connection state management.
While this change appears safe for the current codebase (which uses only EF Core CRUD operations without session-level modifications), it's important to document why this setting is needed. The PR title suggests this "fixes" something, but there's no explanation of what problem this solves.
Consider adding a comment explaining:
- Why this setting is needed (performance optimization or specific issue?)
- What assumptions it makes about connection usage (no temp tables, no session-level settings changes, etc.)
- Any constraints it imposes on future development
This will help future maintainers understand the rationale and avoid introducing code that could be affected by this setting.
| Database = Uri.UnescapeDataString(databaseName), | |
| Database = Uri.UnescapeDataString(databaseName), | |
| // Performance optimization: avoid issuing DISCARD ALL when a connection is returned | |
| // to the pool. This keeps pooled connections "warm" and reduces overhead for simple | |
| // EF Core CRUD operations, which is the only way this application currently uses | |
| // the database. | |
| // | |
| // Assumptions: | |
| // - The application does not rely on session-level state such as: | |
| // - temp tables | |
| // - per-session configuration via SET commands | |
| // - advisory locks or other session-scoped objects | |
| // - Each request is expected to work correctly on a reused connection without | |
| // needing a full session reset. | |
| // | |
| // Constraints for future development: | |
| // - If new code introduces session-level state that must not leak between requests, | |
| // either remove this flag (allowing DISCARD ALL on close) or ensure that such code | |
| // explicitly cleans up or reinitializes its required state. |
Changes
NoResetOnClose = truein connection options