You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/06/16 12:02:57 UTC
[GitHub] [apisix] mikawudi opened a new pull request, #7266: feat: export some importent params for kafka-client
mikawudi opened a new pull request, #7266:
URL: https://github.com/apache/apisix/pull/7266
export 4 importent params about kafka-producter for kafka-logger plugins
if no problem on this commit, i will edit docs later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #7266: feat: export some importent params for kafka-client
Posted by GitBox <gi...@apache.org>.
spacewander merged PR #7266:
URL: https://github.com/apache/apisix/pull/7266
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] mikawudi commented on a diff in pull request #7266: feat: export some importent params for kafka-client
Posted by GitBox <gi...@apache.org>.
mikawudi commented on code in PR #7266:
URL: https://github.com/apache/apisix/pull/7266#discussion_r899940881
##########
apisix/plugins/kafka-logger.lua:
##########
@@ -83,6 +83,11 @@ local schema = {
-- in lua-resty-kafka, cluster_name is defined as number
-- see https://github.com/doujiang24/lua-resty-kafka#new-1
cluster_name = {type = "integer", minimum = 1, default = 1},
+ -- config for lua-resty-kafka, default value is same as lua-resty-kafka
+ kafka_batch_num = {type = "integer", minimum = 1, default = 200},
+ kafka_batch_size = {type = "integer", minimum = 0, default = 1048576},
+ kafka_max_buffering = {type = "integer", minimum = 1, default = 50000},
+ kafka_time_linger = {type = "integer", minimum = 1, default = 1}
Review Comment:
because this params named “linger.ms” in official kafka client producer and other popular kafka client like sarama,
and i don‘t know why named “flush_time” in resty-kafka......i think call it like “linger” is friendly for user google it....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #7266: feat: export some importent params for kafka-client
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7266:
URL: https://github.com/apache/apisix/pull/7266#discussion_r901095840
##########
docs/en/latest/plugins/kafka-logger.md:
##########
@@ -47,6 +47,10 @@ For more info on Batch-Processor in Apache APISIX please refer.
| include_resp_body| boolean | optional | false | [false, true] | Whether to include the response body. The response body is included if and only if it is `true`. |
| include_resp_body_expr | array | optional | | | When `include_resp_body` is true, control the behavior based on the result of the [lua-resty-expr](https://github.com/api7/lua-resty-expr) expression. If present, only log the response body when the result is true. |
| cluster_name | integer | optional | 1 | [0,...] | the name of the cluster. When there are two or more kafka clusters, you can specify different names. And this only works with async producer_type.|
+| producer_batch_num | integer | optional | 200 | [1,...] | `batch_num` param in [lua-resty-kafka],merge message and batch send to server, unit is message count |
Review Comment:
Should be `[lua-resty-kafka](link to lua-resty-kafka)`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #7266: feat: export some importent params for kafka-client
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7266:
URL: https://github.com/apache/apisix/pull/7266#discussion_r899795240
##########
apisix/plugins/kafka-logger.lua:
##########
@@ -83,6 +83,11 @@ local schema = {
-- in lua-resty-kafka, cluster_name is defined as number
-- see https://github.com/doujiang24/lua-resty-kafka#new-1
cluster_name = {type = "integer", minimum = 1, default = 1},
+ -- config for lua-resty-kafka, default value is same as lua-resty-kafka
+ kafka_batch_num = {type = "integer", minimum = 1, default = 200},
Review Comment:
Would be better to use `producer` prefix instead of `kafka`? Since this plugin is already a kafka logger.
##########
apisix/plugins/kafka-logger.lua:
##########
@@ -83,6 +83,11 @@ local schema = {
-- in lua-resty-kafka, cluster_name is defined as number
-- see https://github.com/doujiang24/lua-resty-kafka#new-1
cluster_name = {type = "integer", minimum = 1, default = 1},
+ -- config for lua-resty-kafka, default value is same as lua-resty-kafka
+ kafka_batch_num = {type = "integer", minimum = 1, default = 200},
+ kafka_batch_size = {type = "integer", minimum = 0, default = 1048576},
+ kafka_max_buffering = {type = "integer", minimum = 1, default = 50000},
+ kafka_time_linger = {type = "integer", minimum = 1, default = 1}
Review Comment:
Why don't we call it `flush_time`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] lijing-21 commented on pull request #7266: feat: export some importent params for kafka-client
Posted by GitBox <gi...@apache.org>.
lijing-21 commented on PR #7266:
URL: https://github.com/apache/apisix/pull/7266#issuecomment-1182940108
Hi @mikawudi , thanks for the contribution!
Here is the Contributor T-shirt form[1], if you're interested, kindly take a look :)
[1] https://github.com/apache/apisix/blob/master/CONTRIBUTING.md#contributor-t-shirt
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] mikawudi commented on a diff in pull request #7266: feat: export some importent params for kafka-client
Posted by GitBox <gi...@apache.org>.
mikawudi commented on code in PR #7266:
URL: https://github.com/apache/apisix/pull/7266#discussion_r899931928
##########
apisix/plugins/kafka-logger.lua:
##########
@@ -83,6 +83,11 @@ local schema = {
-- in lua-resty-kafka, cluster_name is defined as number
-- see https://github.com/doujiang24/lua-resty-kafka#new-1
cluster_name = {type = "integer", minimum = 1, default = 1},
+ -- config for lua-resty-kafka, default value is same as lua-resty-kafka
+ kafka_batch_num = {type = "integer", minimum = 1, default = 200},
Review Comment:
good idea,i will change it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org