Skip to content

index: reuse one remote per name when building data index - #11124

Open
MrNK2107 wants to merge 1 commit into
treeverse:mainfrom
MrNK2107:fix/data-status-one-remote-per-name
Open

MrNK2107 wants to merge 1 commit into
treeverse:mainfrom
MrNK2107:fix/data-status-one-remote-per-name

Conversation

@MrNK2107

Copy link
Copy Markdown
Contributor
  • ❗ I have followed the Contributing to DVC checklist.

  • 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here. (No docs change needed.)

Fixes #11121

Index.data called DataCloud.get_remote(out.remote) once per output, and every call builds a new filesystem and Remote. Each Remote stays referenced from the index storage map, so with an empty local cache data status --not-in-remote ends up with one cloud client per directory output, as measured in the issue.

This change resolves each remote name once per index build (a small dict passed from Index.data to _load_storage_from_out) and shares that Remote between all outputs that use it. Outputs with a different remote: still get their own. DataCloud.get_remote itself is unchanged, so other callers (push/pull/gc with a --remote) behave as before.

I chose local memoisation over caching inside DataCloud.get_remote because get_remote is also used with config overrides and by long-lived Repo objects, where a cache would need invalidation on config changes. Happy to move it there if you prefer.

Test: tests/func/test_repo_index.py::test_data_index_reuses_remote_per_name builds 4 directory outputs, 3 on the default remote and 1 on a second remote, and checks that get_remote is called once per name and that the 3 default-remote outputs share one ODB. It fails without the change (['', '', '', 'second'] == ['', 'second']) and passes with it; test_repo_index.py + test_data_status.py pass locally.

Index.data called DataCloud.get_remote once per output, and each call
builds a new filesystem. With many directory outputs and an empty local
cache (fresh clone, CI), `dvc data status --not-in-remote` instantiated
one cloud client per output and memory grew with the number of outputs.

Resolve each remote name once per index build and share the Remote
between the outputs that use it.

Fixes treeverse#11121
@github-project-automation github-project-automation Bot moved this to Backlog in DVC Oct 25, 2026
@codecov

codecov Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.98%. Comparing base (2431ec6) to head (5d98f77).
⚠️ Report is 213 commits behind head on main.

Files with missing lines Patch % Lines
dvc/repo/index.py 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11124      +/-   ##
==========================================
+ Coverage   90.68%   90.98%   +0.29%     
==========================================
  Files         504      505       +1     
  Lines       39795    41158    +1363     
  Branches     3141     3265     +124     
==========================================
+ Hits        36087    37446    +1359     
- Misses       3042     3072      +30     
+ Partials      666      640      -26     

☔ View full report in Codecov by Harness.
📢 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.

@CLAassistant

CLAassistant commented Oct 25, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

This branch has not been deployed

No deployments
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.

data status: --not-in-remote creates an S3 client per output and runs out of memory

2 participants