Skip to content

Fix Npgsql connection pool and update configurations#425

Merged
davideme merged 4 commits intomainfrom
claude/friendly-feistel
Feb 26, 2026
Merged

Fix Npgsql connection pool and update configurations#425
davideme merged 4 commits intomainfrom
claude/friendly-feistel

Conversation

@davideme
Copy link
Owner

@davideme davideme commented Feb 26, 2026

Changes

  • C# Service Configuration: Disable reset on close for Npgsql connection pool by setting NoResetOnClose = true in connection options

Copilot AI review requested due to automatic review settings February 26, 2026 11:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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),
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

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:

  1. Why this setting is needed (performance optimization or specific issue?)
  2. What assumptions it makes about connection usage (no temp tables, no session-level settings changes, etc.)
  3. 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.

Suggested change
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.

Copilot uses AI. Check for mistakes.
@davideme davideme merged commit b8835c7 into main Feb 26, 2026
7 checks passed
@davideme davideme deleted the claude/friendly-feistel branch February 26, 2026 12:06
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.

2 participants