Skip to content
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

Knative service can not scale up with kafka cluster channel provisioner #693

Closed
fatkun opened this issue Dec 24, 2018 · 7 comments
Closed

Comments

@fatkun
Copy link

fatkun commented Dec 24, 2018

Expected Behavior

Knative service scale up after many request.

Actual Behavior

Knative service not scale up.

Steps to Reproduce the Problem

  1. Start a knative service, the service just sleep one second each request.
  2. Use wrk send request to channel

Additional Info

Knative service scale according request concurrency.

Current kafka channel implement:

  1. Channel receive one event, push to kafka topic
  2. kafka consume one event
  3. dispatch message and wait it finish
  4. consume next event

go func() {
for {
msg, more := <-consumer.Messages()
if more {
d.logger.Info("Dispatching a message for subscription", zap.Any("channelRef", channelRef), zap.Any("subscription", sub))
message := fromKafkaMessage(msg)
err := d.dispatchMessage(message, sub)
if err != nil {
d.logger.Warn("Got error trying to dispatch message", zap.Error(err))
}
// TODO: handle errors with pluggable strategy
consumer.MarkOffset(msg, "") // Mark message as processed
} else {
break
}
}
d.logger.Info("Consumer for subscription stopped", zap.Any("channelRef", channelRef), zap.Any("subscription", sub))
}()

There only one concurrency if start one dispatcher pod. It's not enough to scale up knative service.
Can we use multi-goroutine to dispatch message?

@evankanderson
Copy link
Member

evankanderson commented Dec 24, 2018 via email

@fatkun
Copy link
Author

fatkun commented Dec 25, 2018

@evankanderson
I think we can't create too much partition(such as 1000 partition). One event per partition in flight at a time maybe not engouth for scale up service.

removes the ordering guarantees of kafka

I think this is not important, If we create multiple partition, can't guarantee the ordering between partitions.

also complicates the at-least-once delivery guarantees

Yes. Each markOffset should commit previous offset of minimum offset in flight ( min(offsets in flight) - 1 ). There maybe other situation I miss(such as how to handle fail delivery).

@yuzisun
Copy link
Contributor

yuzisun commented Jan 30, 2019

Notice the same issue here, the messages are all dispatched one by one synchronously and there is only one go routine which really limits the scalability, gcp pubsub allows multiple go routines(https://github.com/knative/eventing/blob/master/vendor/cloud.google.com/go/pubsub/subscription.go#L474).

@evankanderson
Copy link
Member

evankanderson commented Jan 30, 2019 via email

@yuzisun
Copy link
Contributor

yuzisun commented Jan 31, 2019

@evankanderson Thanks for the explanation! I got that we need to preserve the ordering but this is needed at partition level, sarama supports cluster.ConsumerModePartitions mode which creates a consumer per partition which potentially increases the parallelism than what we have now, if this something we'd like to support I'd love to send in a PR.

@evankanderson
Copy link
Member

/assign @yuzisun

@knative-prow-robot
Copy link
Contributor

@evankanderson: GitHub didn't allow me to assign the following users: yuzisun.

Note that only knative members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign @yuzisun

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants