-
Notifications
You must be signed in to change notification settings - Fork 242
Event lookup cache #593
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
Event lookup cache #593
Conversation
800e716 to
f8920ed
Compare
Codecov Report
@@ Coverage Diff @@
## main #593 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 305 305
Lines 17983 18006 +23
=========================================
+ Hits 17983 18006 +23
Continue to review full report at Codecov.
|
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
f8920ed to
3af7301
Compare
internal/batch/batch_manager.go
Outdated
| for _, p := range d.processors { | ||
| processors = append(processors, p) | ||
| if !exists[p] { | ||
| processors = append(processors, p) |
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.
Seems fine... but why does it contain duplicates in the first place? Guess we can have a processor shared by more than one dispatcher?
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.
for _, d := range bm.dispatchers {This is iterating over a map, where a dispatcher can register itself multiple times.
I've updated the code to maintain a list as well as a map, to avoid this confusion
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.
Makes sense, thanks for the tweak
awrichar
left a comment
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.
Marking approved with notes to fix a few small items
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
No description provided.