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

Re-check deprecation of setDefaultTopicConf #384

Closed
nick-zh opened this issue Aug 26, 2020 · 5 comments
Closed

Re-check deprecation of setDefaultTopicConf #384

nick-zh opened this issue Aug 26, 2020 · 5 comments
Assignees
Milestone

Comments

@nick-zh
Copy link
Collaborator

nick-zh commented Aug 26, 2020

If seems for the low level consumer, some settings till need to be set over setDefaultTopicConf.
Check in librdkafka if we can get around using rd_kafka_conf_set_default_topic_conf which is said to be deprecated.

@nick-zh nick-zh self-assigned this Aug 26, 2020
@nick-zh nick-zh added this to the v5.0.0 milestone Aug 26, 2020
@nick-zh
Copy link
Collaborator Author

nick-zh commented Sep 9, 2020

Got feedback from the maintainer of librdkafka:

You can set it on the global config object unless you pass a topic config object or a default topic config object.
In those two cases the global config for the topic will be ignored

I will check on our side if this happens somewhere, since it seems like the global config is ignored atm

@nick-zh
Copy link
Collaborator Author

nick-zh commented Sep 14, 2020

So this is the current finding, after setting auto.offset.reset, a dump of the conf object will result in having:

...
    ['default_topic_conf'] = '0x55fd2d9260a0',
...

Which as mentioned above would cause a setting like auto.commit.interval.ms to be ignored on a global level.
Will further investigate if this happens inside librdkafka since i was unable to find code on our side that could cause this to happen.

@nick-zh
Copy link
Collaborator Author

nick-zh commented Sep 15, 2020

auto.commit.interval.ms is a topic configuration for a low level consumer unlike the high level consumer where this is a global setting.
For libraries based on php-rdkafka or pure PHP implementations, this setting must be set on a topic config that can be passed to newTopic.
With this, i think it is fair to remove setDefaultTopicConf for the next major (was deprecated).

@Steveb-p
Copy link
Contributor

@nick-zh we should remember to include this in upgrade path document, if it's created.

@nick-zh
Copy link
Collaborator Author

nick-zh commented Sep 15, 2020

#387

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

No branches or pull requests

2 participants