Skip to content

Conversation

edwin-onyx
Copy link
Contributor

@edwin-onyx edwin-onyx commented Sep 17, 2025

decent optimization would be to remove transformers pkg from the api server requirements

https://linear.app/danswer/issue/DAN-2551/remove-transformers-dependency-from-api-server

docker stats w current req ->
onyx-stack-background-1 6.838GiB
onyx-stack-api_server-1 805MiB

w/o transformers pkg
onyx-stack-background-1 5.788GiB
onyx-stack-api_server-1 718.6MiB

in terms of regression - yuhong:
the logging is not important
I think it's totally fine to just remove entirely

Description

[Provide a brief description of the changes in this PR]

How Has This Been Tested?

[Describe the tests you ran to verify your changes]

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

  • This PR should be backported (make sure to check that the backport attempt succeeds)
  • [Optional] Override Linear Check

Summary by cubic

Remove the transformers dependency and related logging to reduce memory usage in background workers. Add a memory analysis script to inspect container and per-package import memory impact.

  • Dependencies

    • Removed transformers==4.49.0 from backend/requirements/default.txt.
    • Dropped transformers import and warning controls from onyx/natural_language_processing/utils.py.
  • New Features

    • Added backend/memory_summary.py CLI to show process RSS/VMS, Docker stats, supervisord processes, and measure per-package import memory deltas.

- Remove transformers import and logging from utils.py
- Remove transformers==4.49.0 from requirements/default.txt
- Add memory_summary.py for analyzing container memory usage

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link

vercel bot commented Sep 17, 2025

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

Project Deployment Preview Comments Updated (UTC)
internal-search Ready Ready Preview Comment Sep 17, 2025 5:22pm

@edwin-onyx edwin-onyx changed the title transformers dependency fix(infra): remove transformers dependency for api server Sep 17, 2025
@edwin-onyx edwin-onyx marked this pull request as ready for review September 17, 2025 18:21
@edwin-onyx edwin-onyx requested a review from a team as a code owner September 17, 2025 18:21
@edwin-onyx edwin-onyx requested a review from Weves September 17, 2025 18:21
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.

Greptile Summary

This PR implements a memory optimization by removing the transformers library dependency from the API server requirements. The change involves removing transformers==4.49.0 from backend/requirements/default.txt and eliminating the corresponding import and logging configuration from onyx/natural_language_processing/utils.py.

The optimization leverages Onyx's microservice architecture, where ML functionality is properly separated between the API server and model server components. The transformers package was only being used in the API server for non-critical logging configuration (transformer_logging.set_verbosity_error() and warning suppression), while the actual model inference happens in the dedicated model server that retains the transformers dependency in model_server.txt.

This change demonstrates good architectural separation of concerns - the API server handles business logic and API endpoints while the model server manages ML model operations. The memory savings are significant: API server container memory drops from 805MiB to 718.6MiB (~86MiB reduction), and background worker memory decreases from 6.838GiB to 5.788GiB (~1GB reduction).

The remaining tokenization functionality in utils.py continues to work through the tokenizers library and tiktoken, which handle text processing independently of the full transformers stack.

Confidence score: 4/5

  • This PR is safe to merge with low risk as it removes non-critical logging functionality while preserving core features
  • Score reflects clear architectural separation and significant memory benefits, with minimal risk of functional regression
  • Pay close attention to any runtime errors in areas that might have unexpected transformers dependencies

2 files reviewed, no comments

Edit Code Review Bot Settings | Greptile

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

@Weves Weves merged commit f8e9060 into main Sep 18, 2025
63 of 73 checks passed
@Weves Weves deleted the edwin/remove-transformer branch September 18, 2025 19:59
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