refactor: Refactor discovery ModelManager to use parking_lot::RwLock

[paulhendricks]

Overview:

Replaces std::sync::RwLock with parking_lot::RwLock in the discovery module.
This change improves performance and consistency with the rest of the codebase, while removing the need for .unwrap() calls on lock guards.

Details:

• Updated imports to use parking_lot::{Mutex, RwLock}.
• Removed all .unwrap() calls on RwLock guards.
• No functional logic changes — this is strictly an internal refactor for better ergonomics and performance.

Where should the reviewer start?

• lib/llm/src/discovery/model_manager.rs

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

Relates to:

• refactor: Switch ModelManager locks from std::sync::Mutex to parking_lot::Mutex #2696

Summary by CodeRabbit

• New Features

  • Public API now supports registering chat completions and embeddings models, expanding model coverage and simplifying integration.

• Refactor

  • Improved concurrency in model management with more efficient read/write locking, reducing contention and avoiding lock-poisoning issues.
  • Streamlined internal access patterns for listing and retrieving models, resulting in smoother and more reliable operations under load.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

Caution

Review failed

Failed to post review comments.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 85b3dd4 and ce31481.

📒 Files selected for processing (1)

• lib/llm/src/discovery/model_manager.rs (5 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-09-02T16:46:54.015Z

Learnt from: GuanLuo
PR: ai-dynamo/dynamo#2714
File: lib/llm/src/discovery/model_entry.rs:38-42
Timestamp: 2025-09-02T16:46:54.015Z
Learning: In lib/llm/src/discovery/model_entry.rs, GuanLuo prefers not to add serde defaults for model_type and model_input fields to keep the specification explicit and avoid user errors, relying on atomic deployment strategy to avoid backward compatibility issues.

Applied to files:

• lib/llm/src/discovery/model_manager.rs

📚 Learning: 2025-07-16T12:41:12.543Z

Learnt from: grahamking
PR: ai-dynamo/dynamo#1962
File: lib/runtime/src/component/client.rs:270-273
Timestamp: 2025-07-16T12:41:12.543Z
Learning: In lib/runtime/src/component/client.rs, the current mutex usage in get_or_create_dynamic_instance_source is temporary while evaluating whether the mutex can be dropped entirely. The code currently has a race condition between try_lock and lock().await, but this is acknowledged as an interim state during the performance optimization process.

Applied to files:

• lib/llm/src/discovery/model_manager.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build and Test - dynamo
• GitHub Check: pre-merge-rust (.)
• GitHub Check: pre-merge-rust (lib/runtime/examples)
• GitHub Check: pre-merge-rust (lib/bindings/python)

Walkthrough

Migrates ModelManager’s locking from std::sync::RwLock to parking_lot::RwLock, updating read/write access patterns to drop unwraps. Adds two public methods to register chat-completions and embeddings models. Internal engine maps and retrieval paths are updated to use parking_lot guards. No signature changes to existing getters.

Changes

Cohort / File(s)	Summary
Locking primitive migration and API expansion lib/llm/src/discovery/model_manager.rs	Replaced std::sync::RwLock with parking_lot::RwLock; adjusted imports and all read()/write() usages to drop unwraps. Updated engine map accessors accordingly. Added public methods: add_chat_completions_model(...) and add_embeddings_model(...) for registering non-completions engines.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller
  participant MM as ModelManager
  participant CCM as Chat Models Map (RwLock)
  participant EM as Embeddings Map (RwLock)

  rect rgba(200,235,255,0.25)
  note over C,MM: Add chat completions model (new)
  C->>MM: add_chat_completions_model(model, engine)
  MM->>CCM: write()
  activate CCM
  CCM-->>MM: write guard
  MM->>CCM: insert(model, engine)
  CCM-->>MM: drop guard
  deactivate CCM
  MM-->>C: Result<(), Error>
  end

  rect rgba(220,255,220,0.25)
  note over C,MM: Get embeddings engine (existing, lock API changed)
  C->>MM: get_embeddings_engine(model)
  MM->>EM: read()
  activate EM
  EM-->>MM: read guard
  MM->>EM: get(model)
  EM-->>MM: drop guard
  deactivate EM
  MM-->>C: Option/Result<Engine>
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• refactor: Switch ModelManager locks from std::sync::Mutex to parking_lot::Mutex #2696 — Similar synchronization refactor in ModelManager, migrating to parking_lot (focused on Mutex), likely complementary to this RwLock migration.

Poem

A rabbit taps keys with a lock’s gentle click,
From std to parking_lot—swift as a trick.
New paths for chats, embeddings to map,
Read guards glide by without a trap.
Models hop in, engines align—
Concurrency’s chorus, crisp and fine. 🐇✨

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.