-
Notifications
You must be signed in to change notification settings - Fork 368
reader subscribe all partitioned topics #705
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
base: master
Are you sure you want to change the base?
reader subscribe all partitioned topics #705
Conversation
|
cc @cckellogg PTAL |
…nto reader-subscribe-all-partitions
| // Name returns the name of consumer. | ||
| Name() string | ||
|
|
||
| // lastDequeuedMsg used for setting last dequeued msg id by internal partition consumers |
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.
I don't think we should be adding private methods to the public interface. I think these should be removed. Also, it seems these are only relevant to the reader implementation so I think these helper functions should live within the reader files.
| NackBackoffPolicy NackBackoffPolicy | ||
|
|
||
| // startMessageID internally used by multitopic-reader | ||
| startMessageID MessageID |
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 ConsumerOptions is a public interface so I don't think we should add package private fields here. Let's remove these.
| } | ||
|
|
||
| func (pc *partitionConsumer) messagesInQueue() int { | ||
| return len(pc.queueCh) |
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.
If there are multi go routines consuming and producing to this channel this value might not be useful after being read.
| c.handlers.Add(reader) | ||
| return reader, nil | ||
| if len(topics) <= 1 { | ||
| reader, err := newReader(c, options) |
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 different between a reader and a multi reader? It seems like they should be the same but just consume a different number of partitions?
Initial changes to address reader topic not found issue: #553
Changes: