Skip to content

Remove exponential Redis call#7523

Merged
galvana merged 5 commits intomainfrom
asachs/connector-type-redis-fix
Feb 28, 2026
Merged

Remove exponential Redis call#7523
galvana merged 5 commits intomainfrom
asachs/connector-type-redis-fix

Conversation

@adamsachs
Copy link
Copy Markdown
Contributor

@adamsachs adamsachs commented Feb 27, 2026

Description Of Changes

Fixes a severe performance regression in the GET /api/v1/connection endpoint caused by exponential Redis round-trips during secret masking serialization.

Root Cause

When serializing connection configs with secrets, the mask_sensitive_values Pydantic validator calls get_connection_type_secret_schema() for each item. This function previously called get_connection_types() — which rebuilds the entire SaaS connector registry (95 types) via get_saas_connection_types() — just to validate the connection type exists, before returning the schema.

Each of the 95 SaaS types triggers ConnectorRegistry.get_connector_template(), which invokes a Redis GET via the @redis_version_cached decorator. With 50 connection configs per page, this produced:

50 items × 95 SaaS types × ~2ms/Redis call = ~9,500ms per request

Evidence (from plus-rc diagnostic logging in #7514)

Metric Value
DB query 18ms
Serialization (50 items) 9,126ms
Redis calls 4,224 calls
Redis total time 8,893ms
Per-item serialization ~190ms (all in schema_lookup, masking itself: 0.0ms)
Per-item breakdown get_connection_types=~190ms → get_saas_connection_types build_map=~188ms for 95 types

Fix

Short-circuit get_connection_type_secret_schema() to avoid calling get_connection_types() entirely:

  1. DB/non-SaaS types (website, bigquery, mysql, etc.): Check secrets_schemas dict directly — instant O(1) lookup, zero Redis calls
  2. SaaS types: Call ConnectorRegistry.get_connector_template() for just the specific type — 1 Redis call instead of 95
  3. Unknown types: Raise the same NoSuchConnectionTypeSecretSchemaError

Expected Improvement

Metric Before After
Redis calls per page ~4,200 0 (for DB types) or ~50 (for SaaS types)
Serialization time (50 items) ~9,100ms <50ms
Total endpoint time ~9,100ms ~70ms

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
  • No UX review needed
  • No followup issues
  • No migrations
  • No documentation updates required

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Feb 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Feb 27, 2026 11:22pm
fides-privacy-center Ignored Ignored Feb 27, 2026 11:22pm

Request Review

@galvana galvana changed the title [draft] remove exponential redis call Remove exponential Redis call Feb 27, 2026
@galvana galvana marked this pull request as ready for review February 27, 2026 23:42
@galvana galvana requested a review from a team as a code owner February 27, 2026 23:42
@galvana galvana requested review from galvana and removed request for a team February 27, 2026 23:42
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 27, 2026

Greptile Summary

This PR resolves a critical performance regression in the GET /api/v1/connection endpoint by eliminating exponential Redis calls during secret masking serialization.

Key improvements:

  • Reduced Redis calls from ~4,224 to 0-50 per request by implementing a two-tier lookup strategy
  • Improved serialization time from ~9,100ms to <50ms for 50 connection configs
  • Fast path: DB/non-SaaS types use O(1) dictionary lookup from secrets_schemas (zero Redis calls)
  • Slow path: SaaS types call ConnectorRegistry.get_connector_template() for only the specific type (1 Redis call instead of 95)

Additional fixes:

  • manual_webhook connection type now correctly returns its schema (200 with empty properties) instead of incorrectly returning 404
  • https connection type now correctly masks only sensitive fields (authorization, headers) while leaving url unmasked per its schema definition

The implementation is clean, well-tested with comprehensive coverage for edge cases, and the PR description provides excellent metrics and evidence from production diagnostics.

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • Clean performance optimization with comprehensive test coverage, fixes a critical production issue with measurable improvements, and includes proper validation for edge cases. No breaking changes, only bug fixes and performance improvements.
  • No files require special attention

Important Files Changed

Filename Overview
src/fides/api/util/connection_type.py Optimized get_connection_type_secret_schema() to use fast O(1) lookup for DB/non-SaaS types, avoiding 95 Redis calls per request
tests/ops/util/test_connection_type.py Added comprehensive tests for https, manual_webhook, and error cases to validate the fast path optimization

Last reviewed commit: 410c9b5

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@galvana galvana added this pull request to the merge queue Feb 27, 2026
Merged via the queue into main with commit c3a10c9 Feb 28, 2026
54 of 55 checks passed
@galvana galvana deleted the asachs/connector-type-redis-fix branch February 28, 2026 00:03
galvana added a commit that referenced this pull request Feb 28, 2026
Co-authored-by: Nate Smith <nate@ethyca.com>
Co-authored-by: Adrian Galvan <adrian@ethyca.com>
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.

3 participants