Skip to content

Conversation

@JadeCara
Copy link
Contributor

@JadeCara JadeCara commented Nov 14, 2025

Ticket ENG-1985

Description Of Changes

🎯 We are seeing some instability with NYT related to OOM errors on the server.

There are some large columns that have caused memory issues before, we are loading these into memory in several places using db.query(PrivacyRequest) even when they aren’t needed for the response/functionality.

ex: line 818

Created a new classmethod - PrivacyRequest.query_without_large_columns to re-use places where we want to query for PrivacyRequests without going OOM.

Code Changes

  • added new query_without_large_columns class method to PrivacyRequest
  • Used new function to defer large col queries in the following:
    • src/fides/api/api/v1/endpoints/privacy_request_endpoints.py
    • src/fides/api/service/privacy_request/email_batch_service.py
    • src/fides/api/service/privacy_request/request_service.py
    • src/fides/api/util/fuzzy_search_utils.py

Steps to Confirm

  1. Spin up FidesPlus pointed at this branch - run several DSRs to make sure everything still runs as expected.
  2. Go to the manage requests tab and make sure things load without errors, also click into several tasks to make sure there are no loading errors there.
  3. Try searching through requests verify no errors.

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link

vercel bot commented Nov 14, 2025

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

Project Deployment Preview Comments Updated (UTC)
fides-plus-nightly Ready Ready Preview Comment Nov 17, 2025 11:19pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
fides-privacy-center Ignored Ignored Nov 17, 2025 11:19pm

@codecov
Copy link

codecov bot commented Nov 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.31%. Comparing base (7ff64b1) to head (50d9a93).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6975   +/-   ##
=======================================
  Coverage   87.31%   87.31%           
=======================================
  Files         525      525           
  Lines       34512    34515    +3     
  Branches     3984     3984           
=======================================
+ Hits        30134    30137    +3     
  Misses       3511     3511           
  Partials      867      867           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JadeCara JadeCara marked this pull request as ready for review November 14, 2025 18:21
@JadeCara JadeCara requested a review from a team as a code owner November 14, 2025 18:21
@JadeCara JadeCara requested review from vcruces and removed request for a team November 14, 2025 18:21
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 14, 2025

Greptile Overview

Greptile Summary

This PR introduces a memory optimization to prevent OOM errors when querying multiple privacy requests. A new query_without_large_columns classmethod was added to the PrivacyRequest model that uses SQLAlchemy's defer() to avoid loading the _filtered_final_upload and access_result_urls columns until explicitly accessed.

The optimization is applied in five strategic locations:

  • Privacy request list/search endpoint for paginated results
  • Batch email processing for requests awaiting email send
  • Polling for exited privacy request tasks
  • Requeuing interrupted tasks
  • Building the fuzzy search automaton

All modified queries only need basic request metadata and identity data, making them ideal candidates for deferred loading. Individual privacy request queries (by ID) continue to load all columns normally, ensuring existing functionality for accessing filtered results and URLs remains intact.

Confidence Score: 5/5

  • This PR is safe to merge - it's a focused performance optimization with minimal risk
  • The implementation correctly uses SQLAlchemy's defer() for lazy loading, which is a standard pattern. All modified query locations were verified to not access the deferred columns, and individual lookups by ID remain unchanged. The approach is well-documented and addresses a production OOM issue without breaking existing functionality.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/fides/api/models/privacy_request/privacy_request.py 5/5 Adds new classmethod query_without_large_columns that defers loading of _filtered_final_upload and access_result_urls columns to prevent OOM errors in list queries
src/fides/api/api/v1/endpoints/privacy_request_endpoints.py 5/5 Replaces db.query(PrivacyRequest) with query_without_large_columns in list endpoint to prevent loading large columns unnecessarily
src/fides/api/service/privacy_request/request_service.py 5/5 Uses query_without_large_columns in poll_for_exited_privacy_request_tasks and requeue_interrupted_tasks for efficient bulk request processing

Copy link
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.

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@vcruces vcruces left a comment

Choose a reason for hiding this comment

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

The code looks great!

I tested by creating 4 privacy requests and applied actions like approve, deny, and delete. I also tried the search bar and it worked correctly. Nice work!

@JadeCara JadeCara enabled auto-merge November 17, 2025 19:15
@JadeCara JadeCara disabled auto-merge November 17, 2025 19:57
@JadeCara JadeCara enabled auto-merge November 17, 2025 23:15
@JadeCara JadeCara added this pull request to the merge queue Nov 17, 2025
Merged via the queue into main with commit 8b6e430 Nov 18, 2025
69 checks passed
@JadeCara JadeCara deleted the ENG-1985 branch November 18, 2025 00:25
Kelsey-Ethyca pushed a commit that referenced this pull request Nov 18, 2025
Co-authored-by: Jade Wibbels <jade@ethyca.com>
jjdaurora pushed a commit that referenced this pull request Dec 5, 2025
Co-authored-by: Jade Wibbels <jade@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