-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Change uid intersection policy in some cases #8919
base: main
Are you sure you want to change the base?
Conversation
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.
Minor note - possible dead code.
@@ -875,6 +878,21 @@ func (l *List) Rollup(alloc *z.Allocator, readTs uint64) ([]*bpb.KV, error) { | |||
return kvs, nil | |||
} | |||
|
|||
func (l *List) CreateUnpacked() { |
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.
Looks like all callers to this are commented out. Is it dead code?
@@ -71,6 +71,7 @@ const ( | |||
// List stores the in-memory representation of a posting list. | |||
type List struct { | |||
x.SafeMutex | |||
immutable *pb.List |
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 you add a comment that this is unpacked posting list?
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.
Looks like this PR has more changes than just the uid intersection policy changes. Does it also include changes to read multiple keys using the new batch api? If yes, maybe you could split it into two separate PRs.
if err != nil { | ||
return err | ||
} | ||
pickMultiplePostings := q.ExpandAll || (listType && len(q.Langs) == 0) |
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.
Didn't quite understand this block of changes.
Title format:
Topic(Area): Feature
Topic
must be one ofbuild|ci|docs|feat|fix|perf|refactor|chore|test
Area
must be one ofacl|audit|backup|badger|cdc|dql|export|graphql|indexing|multi-tenancy|raft|restore|upgrade|zero
Body Format:
Description:
Fixes:
Closes:
Docs:
LDBC Results. Upto 13.46% better
Benchmark results: