Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix/Removed object listing #2526

Merged
merged 6 commits into from
Aug 29, 2023

Conversation

carpawell
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #2526 (7b472a0) into master (33e6204) will increase coverage by 0.00%.
The diff coverage is 80.00%.

❗ Current head 7b472a0 differs from pull request most recent head 3ec865e. Consider uploading reports for the commit 3ec865e to get more accurate results

@@           Coverage Diff           @@
##           master    #2526   +/-   ##
=======================================
  Coverage   29.68%   29.69%           
=======================================
  Files         404      404           
  Lines       30711    30718    +7     
=======================================
+ Hits         9117     9122    +5     
- Misses      20825    20826    +1     
- Partials      769      770    +1     
Files Changed Coverage Δ
pkg/local_object_storage/metabase/exists.go 85.88% <80.00%> (-1.30%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@carpawell carpawell force-pushed the fix/removed-object-listing branch 3 times, most recently from 3e9bc4b to 0eb8342 Compare August 24, 2023 17:04
@@ -121,7 +121,7 @@ func (db *DB) selectObjects(tx *bbolt.Tx, cnr cid.ID, fs object.SearchFilters, c
addr.SetContainer(cnr)
addr.SetObject(id)

if objectStatus(tx, addr, currEpoch) > 0 {
if objectStatus(tx, addr, currEpoch) > 0 && !objectLocked(tx, cnr, id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

did you think about return specific status from objectStatus for locked objects?

Copy link
Member Author

Choose a reason for hiding this comment

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

what is your idea exactly? in search operations be ready to return objects and status code if any locked objects have been found?

Copy link
Member Author

@carpawell carpawell Aug 25, 2023

Choose a reason for hiding this comment

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

oh, sorry, got you wrong

Copy link
Member Author

@carpawell carpawell Aug 25, 2023

Choose a reason for hiding this comment

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

found your idea usefull (but current codebase does not require a separate code for removed-but-locked objects, so 0 is ok for now), check, please

@carpawell carpawell force-pushed the fix/removed-object-listing branch 2 times, most recently from 64aff79 to 4f133ae Compare August 25, 2023 09:36
CHANGELOG.md Show resolved Hide resolved
pkg/local_object_storage/metabase/exists.go Show resolved Hide resolved
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Also includes a test for removed but locked objects. Refs nspcc-dev#2376.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Refs nspcc-dev#2376.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
It makes logic more reliable if a single function is responsible for the
removed/expired/locked statuses.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
@roman-khimov roman-khimov merged commit fb7dd3f into nspcc-dev:master Aug 29, 2023
7 of 8 checks passed
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