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

Engine Get logic shouldn't depend on shard internals #3024

Open
roman-khimov opened this issue Nov 21, 2024 · 1 comment
Open

Engine Get logic shouldn't depend on shard internals #3024

roman-khimov opened this issue Nov 21, 2024 · 1 comment
Labels
enhancement Improving existing functionality I4 No visible changes neofs-storage Storage node application issues S2 Regular significance U4 Nothing urgent

Comments

@roman-khimov
Copy link
Member

Is your feature request related to a problem? Please describe.

I'm always frustrated when I look at the code added in #1199 to fix #1186:

  • it's very specific, the case of "meta exists elsewhere" is handled, but things like "meta doesn't exist while the object is present" is no
  • it leaks shard internals to engine, engine should not care about metabases, it just tries to Get something
  • it adds some overhead

Describe the solution you'd like

I'd like to revert it and get back to the simple logic of:

  • either we have a metabase and then we trust it
  • or we never trust it for Gets and always get down to the blobstor itself

The answer is not that obvious, testing against the blobstor is expensive, but most of the time we expect it to succeed anyway because we're going to the most probable shard and with fstree it's basically an FS path check, a single syscall. Going into meta is some memory reads which is cheaper in general, but meta is a contention point for many operations, so what happens under which load is an open question.

Describe alternatives you've considered

Corrupted meta can be fixed in the background, btw. We have background routines that go over data, they can check meta consistency and fix it (both ways, get missing object if we have meta or add meta if we have an object).

@roman-khimov roman-khimov added enhancement Improving existing functionality neofs-storage Storage node application issues U4 Nothing urgent S2 Regular significance I4 No visible changes labels Nov 21, 2024
roman-khimov added a commit that referenced this issue Nov 22, 2024
If we have no error and got a result -- no one cares about HasMeta(). If
we've got an error then currently some codes cares about meta status, but it
can be passed as an additional error as well. Related to #3024, in reality
meta should be internal to the shard, but this change at least avoids
excessive booleans.

Signed-off-by: Roman Khimov <[email protected]>
roman-khimov added a commit that referenced this issue Nov 22, 2024
If we have no error and got a result -- no one cares about HasMeta(). If
we've got an error then currently some codes cares about meta status, but it
can be passed as an additional error as well. Related to #3024, in reality
meta should be internal to the shard, but this change at least avoids
excessive booleans.

Signed-off-by: Roman Khimov <[email protected]>
roman-khimov added a commit that referenced this issue Nov 22, 2024
If we have no error and got a result -- no one cares about HasMeta(). If
we've got an error then currently some codes cares about meta status, but it
can be passed as an additional error as well. Related to #3024, in reality
meta should be internal to the shard, but this change at least avoids
excessive booleans.

Signed-off-by: Roman Khimov <[email protected]>
@roman-khimov
Copy link
Member Author

Related to #394.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving existing functionality I4 No visible changes neofs-storage Storage node application issues S2 Regular significance U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

1 participant