-
Notifications
You must be signed in to change notification settings - Fork 238
feat: support range query for per-account blobs at metadata store #1438
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
Conversation
}, | ||
}, | ||
Projection: &types.Projection{ | ||
ProjectionType: types.ProjectionTypeAll, |
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.
All the secondary indexes here are projecting all (hence duplicating data multiple times in aggregate), I wonder if any of these will be better to just projecting just the essential/minimum fields.
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.
Depends on the costs of projecting all. I don't think it's an issue as of right now.
ping |
@@ -32,6 +33,7 @@ const ( | |||
OperatorResponseIndexName = "OperatorResponseIndex" | |||
RequestedAtIndexName = "RequestedAtIndex" | |||
AttestedAtIndexName = "AttestedAtAIndex" | |||
AccountBlobIndexName = "AccountBlobIndex" |
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.
Needs an infrastructure change to add this index?
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.
yea, having a local change which will be sent out later
// | ||
// If limit > 0, returns at most that many blobs. If limit <= 0, returns all results | ||
// in the time range. | ||
func (s *BlobMetadataStore) GetBlobMetadataByAccountID( |
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.
GetBlobMetadataByAccountID looks similar to GetDispersalRequestByDispersedAt, could we have some helper or better to keep the logic separate?
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.
It'll be nice to have a template or something, but just helping two functions seems not that high gain to take the complexity of template yet
Why are these changes needed?
Querying blob feed posted by a given account
Checks