Simplify engine interface #3008
Conversation
These APIs seem to be totally unused. Signed-off-by: Roman Khimov <[email protected]>
We _never_ used non-forced deletion, so only a forced version is provided now. But maybe we can just Inhume(). Signed-off-by: Roman Khimov <[email protected]>
Signed-off-by: Roman Khimov <[email protected]>
No Prm/Res and ignore option is dropped too since there were no users of it. Signed-off-by: Roman Khimov <[email protected]>
Pure refactoring, no functional changes. Signed-off-by: Roman Khimov <[email protected]>
Signed-off-by: Roman Khimov <[email protected]>
Signed-off-by: Roman Khimov <[email protected]>
I believe it's more correct, since an object can be locked, but if we're gholding a copy no one needs it still should go away. Signed-off-by: Roman Khimov <[email protected]>
Both can check for locked status, but this doesn't make much sense, reuse code. Signed-off-by: Roman Khimov <[email protected]>
Delete is a forced removal of a given address, use it this way. Signed-off-by: Roman Khimov <[email protected]>
It's not used by any real code, Delete() is sufficient and Inhume can now be used for tombstone handling only. Signed-off-by: Roman Khimov <[email protected]>
It's specifically for tombstones now since other deletion cases work via Delete(). Signed-off-by: Roman Khimov <[email protected]>
Signed-off-by: Roman Khimov <[email protected]>
Can be split into two methods in future, but works fine for now. Signed-off-by: Roman Khimov <[email protected]>
I believe these should be counted irrespective of blocked/unblocked state, doing this in internal functions is also error-prone since we can count for some unrelated calls as well. Signed-off-by: Roman Khimov <[email protected]>
The only reason they existed is because StorageEngine interfaces were unusable. Now they're not and these wrappers are trivial and do nothing in most of the cases. Signed-off-by: Roman Khimov <[email protected]>
roman-khimov
added
enhancement
roman-khimov
requested review from
carpawell and
cthulhu-rider
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3008 +/- ##
==========================================
- Coverage 23.06% 23.01% -0.06%
==========================================
Files 790 790
Lines 58709 58500 -209
==========================================
- Hits 13542 13461 -81
+ Misses 44285 44161 -124
+ Partials 882 878 -4 ☔ View full report in Codecov by Sentry. |
| inhumePrm.WithForceRemoval() | ||
|
|
||
| _, err := ls.Inhume(inhumePrm) | ||
| err := ls.Delete(addr) |
There was a problem hiding this comment.
now, there is more than one thing that can delete objects, not sure it is expected. GC can be controlled, GC can be tuned or even stopped to prevent unnecessary disk load (#2992) but with this PR there is also a policer that does things on its own
There was a problem hiding this comment.
It did the same thing previously. The only difference is force, that's fa47327.
There was a problem hiding this comment.
i meant deletion through the garbage bucket is more predictable to me. looking at this change makes me think we do not need the garbage bucket at all then
There was a problem hiding this comment.
The logic is the same, at least.
| @@ -20,6 +20,10 @@ func (e *StorageEngine) ContainerSize(cnr cid.ID) (uint64, error) { | |||
| err error | |||
| size uint64 | |||
| ) | |||
| if e.metrics != nil { | |||
There was a problem hiding this comment.
so this changed the game: it is not a "load hardware" metric but a logic one now? at least, i would add this to CHANGELOG
There was a problem hiding this comment.
We completely missed invocations previously. For a normal engine this doesn't change anything. For blocked (closed? I've not found any way to block, really) it'd at least count invocations (time is meaningless in this case).
There was a problem hiding this comment.
closed? I've not found any way to block, really
should be dropped then or try to write code keeping in mind that it can be blocked after some future feature
There was a problem hiding this comment.
It was itching for me as well, but I've kept it for now.
There was a problem hiding this comment.
I would wait for https://github.com/nspcc-dev/neofs-node/actions/runs/11800054691
|
Green. |
This continues #3008 with internal refactoring that is mostly about making code simpler and dropping useless code.
Mostly similar to #3001, we have a number of Prm/Res structures that make it harder to used engine and trace things going on. But there are some functional changes here as well and also things that go beyond Prm/Res. Overall we get rid of many pointless conversions.