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.

Sign up for GitHub

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

Jump to bottom

Store object metadata in metabases for SearchV2 service #3080

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Store object metadata in metabases for SearchV2 service #3080

wants to merge 1 commit into from
+2,259 −2

Conversation

cthulhu-rider
Copy link
Contributor

@cthulhu-rider cthulhu-rider commented Jan 13, 2025 •
edited
Loading

  • store metadata on Put
  • unfiltered
  • test scenarios for filtered
  • filtered
  • attributes
  • test sorting
  • removal
  • migration

@cthulhu-rider cthulhu-rider force-pushed the object-searchv2/metabases branch 4 times, most recently from 9624b00 to 3540889 Compare January 14, 2025 13:17
@cthulhu-rider cthulhu-rider force-pushed the object-searchv2/metabases branch 2 times, most recently from f16c1c1 to 8957af6 Compare January 16, 2025 15:38
roman-khimov
roman-khimov reviewed Jan 16, 2025
View reviewed changes
pkg/local_object_storage/metabase/metadata.go
)

var (
maxUint256 = new(big.Int).SetBytes([]byte{255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255,
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it 127, 255, 255...? At least VM has signed 256-bit integers.

Copy link
Member

Choose a reason for hiding this comment

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

BTW, https://github.com/holiman/uint256 can be useful here, it's much faster than elastic big.Int (refs nspcc-dev/neo-go#1581).

pkg/local_object_storage/metabase/metadata.go Outdated Show resolved Hide resolved
pkg/local_object_storage/metabase/metadata.go Outdated
metaPrefixA = byte(iota)
metaPrefixBI // integer attributes
metaPrefixBS // all other attributes
metaPrefixC
Copy link
Member

Choose a reason for hiding this comment

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

These better be named semantically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for sure, will do this finally. But for, now I'm used to naming from the task

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

pkg/local_object_storage/metabase/metadata.go Outdated
var k []byte
// TODO: move to global funcs
makeKeyB := func(prefix byte, attr string, valLen int) (int, int) {
ln := 1 + oid.Size + len(attr) + valLen + len(utf8Delimiter)*2 // TODO: constantize some stuff
Copy link
Member

Choose a reason for hiding this comment

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

slices.Concat()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i thought about this. Will make code improvement after functionaly stabilization if u dont mind

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added key buffering, concatenation does not fit him

pkg/local_object_storage/metabase/metadata.go Outdated Show resolved Hide resolved
@cthulhu-rider cthulhu-rider force-pushed the object-searchv2/metabases branch 3 times, most recently from c29b7e5 to 00c7b83 Compare January 20, 2025 13:56
@carpawell carpawell mentioned this pull request Jan 20, 2025
Open
@cthulhu-rider cthulhu-rider force-pushed the object-searchv2/metabases branch 5 times, most recently from 22664c2 to 4bb5abf Compare January 24, 2025 11:30
@cthulhu-rider cthulhu-rider force-pushed the object-searchv2/metabases branch 3 times, most recently from c2c593b to 0904251 Compare February 5, 2025 14:10
@cthulhu-rider
Copy link
Contributor Author

it seems working except case when primary filtered attribute is not the 1st requested one. Querying the primary attribute is almost always needed, e.g. with NE/PREFIX/NUM filters. It's only redundant with EQ, but for now a primary attribute request can be required. In the future, if the requirement is relaxed, the client's behavior will not be broken and he will be able to slightly optimize the search query

i also dont plan to support migration and GC in this PR so to not block testing with fresh storages

i need some time to review TODOs, fix linters and beautify the code, but it is rdy overall

@cthulhu-rider cthulhu-rider force-pushed the object-searchv2/metabases branch 3 times, most recently from eb9036f to 871972a Compare February 6, 2025 11:03
@codecov Codecov
Copy link

codecov bot commented Feb 6, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 70.94017% with 170 lines in your changes missing coverage. Please review.

Project coverage is 22.96%. Comparing base (5279df2) to head (5d513b6).
Report is 13 commits behind head on master.

Files with missing lines Patch % Lines
pkg/local_object_storage/metabase/metadata.go 71.48% 105 Missing and 53 partials ⚠️
pkg/local_object_storage/metabase/put.go 55.00% 6 Missing and 3 partials ⚠️
pkg/local_object_storage/metabase/containers.go 0.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3080      +/-   ##
==========================================
+ Coverage   22.47%   22.96%   +0.49%     
==========================================
  Files         751      752       +1     
  Lines       57804    58387     +583     
==========================================
+ Hits        12991    13409     +418     
- Misses      43929    44038     +109     
- Partials      884      940      +56

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cthulhu-rider cthulhu-rider force-pushed the object-searchv2/metabases branch 2 times, most recently from 374e985 to 8dffd53 Compare February 6, 2025 11:49
@cthulhu-rider
node/metabase: Provide API for SearchV2
5d513b6 
There is a need to serve `ObjectService.SearchV2` RPC by the SN. In
order not to expand the structure and configuration of the node, the
best place to store metadata is metabase.

Metabases are extended with per-container object metadata buckets. For
each object, following indexes are created:
 - OID;
 - attribute->OID;
 - OID->attribute.

Integers are stored specifically to reach lexicographic comparisons
without decoding.

New `Search` method is provided: it allows to filter out container's
objects and receive specified attributes. Count is also limited, op is
paged via cursor. In other words, the method follows SearchV2 behavior
within single metabase.

Refs #3058.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider cthulhu-rider force-pushed the object-searchv2/metabases branch from 8dffd53 to 5d513b6 Compare February 6, 2025 11:50
@cthulhu-rider cthulhu-rider mentioned this pull request Feb 6, 2025
Open
@cthulhu-rider cthulhu-rider marked this pull request as ready for review February 6, 2025 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carpawell carpawell Awaiting requested review from carpawell carpawell is a code owner

@roman-khimov roman-khimov Awaiting requested review from roman-khimov roman-khimov is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cthulhu-rider @roman-khimov