[mtsource] MaxResponseSize considers the number of connected brokers #682
Conversation
knative-prow-robot
added
approved
lionelvillard
force-pushed
the
maxresponsesizeperbroker
branch
from
1728b89
to
4564888
Compare
|
The following is the coverage report on the affected files.
|
Codecov Report
@@ Coverage Diff @@
## main #682 +/- ##
==========================================
+ Coverage 75.21% 75.35% +0.13%
==========================================
Files 132 132
Lines 5782 5785 +3
==========================================
+ Hits 4349 4359 +10
+ Misses 1213 1206 -7
Partials 220 220
Continue to review full report at Codecov.
|
|
/test pull-knative-sandbox-eventing-kafka-integration-test-mt-source |
|
/test pull-knative-sandbox-eventing-kafka-upgrade-tests |
| // maxResponseSize is per connected brokers. | ||
| // For now cap the number of connected brokers to maxBrokers until a better solution | ||
| // comes along | ||
| maxResponseSize = int32(float32(maxResponseSize) / float32(maxBrokers)) |
There was a problem hiding this comment.
maxBrokers is not fixed at 6. If someone uses a Kafka instance with 12 brokers, then this would be a problem. I guess for now, we can hard code it as 6. But this value is really per Kafka source. It should go onto the CR.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lionelvillard, steven0711dong The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
#668 followup.
This is a quick fix until a proper solution is implemented.
Proposed Changes
Release Note
Docs