Skip to content

metabase: drop useless tomoveit bucket#3763

Merged
roman-khimov merged 2 commits intomasterfrom
movable-bucket
Dec 29, 2025
Merged

metabase: drop useless tomoveit bucket#3763
roman-khimov merged 2 commits intomasterfrom
movable-bucket

Conversation

@roman-khimov
Copy link
Member

  1. Nothing reads the data from it (status not counted).
  2. It's only set for a very specific case.
  3. It requires an additional DB transaction.
  4. In a dynamic shard environment this doesn't make much sense conceptually, mark can be obsolete.
  5. Proper solution is kinda local policer that should cover a bit more cases, subject of After replication of object to another shard, the object remains on the returned disk forever #2087.

@codecov
Copy link

codecov bot commented Dec 29, 2025

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.79%. Comparing base (f0e646f) to head (6a6f162).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
pkg/local_object_storage/engine/put.go 62.50% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3763      +/-   ##
==========================================
- Coverage   25.89%   25.79%   -0.11%     
==========================================
  Files         659      657       -2     
  Lines       42305    42175     -130     
==========================================
- Hits        10953    10877      -76     
+ Misses      30349    30308      -41     
+ Partials     1003      990      -13     

☔ 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.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the unused "tomoveit bucket" functionality from the metabase, which was used to mark objects for relocation to another shard. As described in the PR description, this feature is being removed because:

  1. No code actually reads the data (except for status queries)
  2. It's only set in a very specific case (when an object already exists on a non-primary shard)
  3. It requires an additional DB transaction
  4. In a dynamic shard environment, these marks can become obsolete

Key changes:

  • Removed the tomoveit bucket and all related database operations (ToMoveIt, DoNotMove, Movable methods)
  • Simplified object placement logic in the storage engine by removing index-based shard movement tracking
  • Updated metabase migration to version 9 to delete the obsolete tomoveit bucket

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/local_object_storage/shard/move.go Deleted file containing shard-level ToMoveIt method for marking objects for relocation
pkg/local_object_storage/metabase/movable.go Deleted file containing metabase operations for managing movable objects
pkg/local_object_storage/metabase/movable_test.go Deleted tests for movable object functionality
pkg/local_object_storage/metabase/version.go Added tomoveit bucket to list of obsolete buckets deleted in version 9 migration
pkg/local_object_storage/metabase/util.go Renamed toMoveItPrefix to unusedToMoveItPrefix and removed toMoveItBucketName variable
pkg/local_object_storage/metabase/status.go Removed bucket reading logic from readBuckets function (now only returns HeaderField data)
pkg/local_object_storage/metabase/status_test.go Removed test case for "moved" object status
pkg/local_object_storage/metabase/metadata.go Removed FilterType case that called delUniqueIndexes during metadata deletion
pkg/local_object_storage/metabase/delete.go Removed delUniqueIndexes helper function that cleaned tomoveit bucket
pkg/local_object_storage/metabase/delete_test.go Removed test assertions checking tomoveit index during deletion
pkg/local_object_storage/metabase/control.go Removed tomoveit bucket from static buckets list
pkg/local_object_storage/metabase/containers_test.go Removed ToMoveIt test case
pkg/local_object_storage/metabase/VERSION.md Updated documentation to reflect removal of tomoveit bucket in version 9
pkg/local_object_storage/engine/put.go Removed shard index parameter and ToMoveIt marking logic when objects already exist on non-primary shards
pkg/local_object_storage/engine/evacuate.go Removed shard index parameter from putToShard calls during evacuation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@roman-khimov roman-khimov force-pushed the movable-bucket branch 4 times, most recently from 0e456ef to 7fb6f4a Compare December 29, 2025 10:24
1. Nothing reads the data from it (status not counted).
2. It's only set for a very specific case.
3. It requires an additional DB transaction.
4. In a dynamic shard environment this doesn't make much sense conceptually,
   mark can be obsolete.
5. Proper solution is kinda local policer that should cover a bit more cases,
   subject of #2087.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Defect of 0ddcf2b.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@roman-khimov roman-khimov merged commit 2503617 into master Dec 29, 2025
21 of 22 checks passed
@roman-khimov roman-khimov deleted the movable-bucket branch December 29, 2025 12:00
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