-
Notifications
You must be signed in to change notification settings - Fork 37
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
Store object metadata in metabases for SearchV2 service #3080
Conversation
cthulhu-rider
commented
Jan 13, 2025
•
edited
Loading
edited
- store metadata on Put
- unfiltered
- test scenarios for filtered
- filtered
- attributes
- test sorting
- removal
- migration
9624b00
to
3540889
Compare
f16c1c1
to
8957af6
Compare
c29b7e5
to
00c7b83
Compare
22664c2
to
4bb5abf
Compare
c2c593b
to
0904251
Compare
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 |
eb9036f
to
871972a
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3080 +/- ##
==========================================
+ Coverage 22.47% 22.92% +0.45%
==========================================
Files 751 750 -1
Lines 57804 58710 +906
==========================================
+ Hits 12991 13462 +471
- Misses 43929 44308 +379
- Partials 884 940 +56 ☔ View full report in Codecov by Sentry. |
8dffd53
to
5d513b6
Compare
5d513b6
to
400c685
Compare
return nil, "", fmt.Errorf("empty attribute #%d", i) | ||
} | ||
if attrs[i] == object.FilterContainerID || attrs[i] == object.FilterID { | ||
return nil, "", fmt.Errorf("prohibited attribute %s", attrs[i]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These checks (including the ones above of course) need to be separated out, they're about semantic request validation, so the Search
handler should perform them first and if successful run searches everywhere (including local) that could fail only for some internal (not related to request itself) reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dropped
primSeekPrefix = primSeekKey[:1+len(primAttr)+len(utf8Delimiter)] | ||
valID := primSeekKey[len(primSeekPrefix):] | ||
if len(valID) <= oid.Size { | ||
return nil, nil, fmt.Errorf("%w: too small VAL_OID len %d", errInvalidCursor, len(valID)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also a part of the initial search parameter check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible with #3080 (comment). Done
|
||
// Search selects up to count container's objects from the given container | ||
// matching the specified filters. | ||
func (db *DB) Search(cnr cid.ID, fs object.SearchFilters, attrs []string, cursor string, count uint16) ([]client.SearchResultItem, string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should meta
package use SDK's client
package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SDK is a basest in the node
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sdk in general -- agree, but the client
package -- not sure
8085434
to
c59f430
Compare
primMatcher == object.MatchNumGT || primMatcher == object.MatchNumGE { | ||
var err error | ||
if primSeekKey, primSeekPrefix, err = seekKeyForAttribute(primAttr, fs[0].Value()); err != nil { | ||
return nil, nil, fmt.Errorf("invalid primary filter value: %w", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be checked before going into searchInBucket
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be, dont think this is sufficient for now. I already have TODO bout this
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 <[email protected]>
c59f430
to
9e2e544
Compare
if s == "" { | ||
return nil, nil | ||
} | ||
b := make([]byte, 1+base64.StdEncoding.DecodedLen(len(s))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isnt DecodeString
about the same but shorter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DecodeString
creates new buffer on his own, this isnt desired here
|
||
// Search selects up to count container's objects from the given container | ||
// matching the specified filters. | ||
func (db *DB) Search(cnr cid.ID, fs object.SearchFilters, attrs []string, cursor string, count uint16) ([]client.SearchResultItem, string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sdk in general -- agree, but the client
package -- not sure