-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Update stream framework with new alternate keyToList function #2211
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
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.
We should add a test as well and considering getting rid of the boolean field that decides what function to use. We can simply check whether a field is nil.
// | ||
// Note: Calls to KeyToList are concurrent. | ||
KeyToList func(key []byte, itr *Iterator) (*pb.KVList, error) | ||
// UseKeyToListWithThreadId is used to indicate that KeyToListWithThreadId should be used |
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.
This comment needs to be simplified.
if kvs, err := st.FinishThread(threadId); err != nil { | ||
return err | ||
} else { | ||
for _, kv := range kvs.Kv { |
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.
we should define this as a default function and assign it to FinishThread
var list *pb.KVList | ||
var err error | ||
if st.UseKeyToListWithThreadId { | ||
list, err = st.KeyToListWithThreadId(item.KeyCopy(nil), itr, threadId) |
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.
what's the benefit of passing the threadId?
No description provided.