-
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
Metadata bucket and GC #3123
Metadata bucket and GC #3123
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3123 +/- ##
==========================================
+ Coverage 22.94% 22.98% +0.03%
==========================================
Files 750 750
Lines 58718 58759 +41
==========================================
+ Hits 13473 13505 +32
- Misses 44299 44304 +5
- Partials 946 950 +4 ☔ View full report in Codecov by Sentry. |
abe8a36
to
8221ff4
Compare
8221ff4
to
68aac77
Compare
kAttrID[0] = metaPrefixAttrIDPlain | ||
copy(kAttrID[1:], kIDAttr[1+oid.Size:]) | ||
copy(kAttrID[len(kAttrID)-oid.Size:], id[:]) | ||
if val := kIDAttr[sepInd+utf8DelimiterLen:]; len(val) == intValLen && val[0] <= 1 { // most likely integer |
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.
And if it's not an int it'll be OK anyway? I'm a bit surprised that IDAttr stores numerics in this format as well. Although it should be OK since we're pretty strict on what is a number and treat requests in the same way attributes are treated.
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.
And if it's not an int it'll be OK anyway?
yes cuz kAttrID
is added anyway. If non-int, then kAttrIDInt
does not exist and Delete()
is nop
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.
urprised that IDAttr stores numerics in this format
that's a good point. I thought about this again, and seems like it'd be more convenient to store in text format:
- attributes are collected as text, so easier to get
- secondary filters will be numerical less often
- numbers approaching max uint64 will be quite rare in practice. Larger ones billion times rarer. Max64 is 20B len in text format while we take 33B for each integer
according to this, i think storing metaPrefixIDAttr
attributes in text format gonna be more efficient. What do u think?
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.
The original intent was to keep attribute contents in IDAttr index as is. If this doesn't break anything then I'd prefer this. If we need to do <>
numeric comparisons we can convert on the fly, but mostly it'd be about exact matches, <>
is more relevant for the primary attribute.
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.
gonna do within #3130
Continues 9e2e544. * Do not include objects with GC/tombstone mark similar to old search. * Remove data from metadata bucket on physical removal of the object. Closes #3118. Signed-off-by: Leonard Lyubich <[email protected]>
68aac77
to
1c27d8d
Compare
No description provided.