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

Breaking change in topic auto creation behvaiour #323

Open
mrahs opened this issue May 12, 2021 · 5 comments
Open

Breaking change in topic auto creation behvaiour #323

mrahs opened this issue May 12, 2021 · 5 comments

Comments

@mrahs
Copy link
Contributor

mrahs commented May 12, 2021

This commit efeaf69 introduced a breaking change by actively avoiding creating new topics, regardless of the broker configurations. It breaks integration tests which rely on creating topics automatically. Without it, every integration test needs to be aware of all possible topics used in the participating services, which is hard to maintain.

It would be great to revert to the original behaviour and make the new one available via a config value in TopicManagerConfig. Something like AvoidAutoCreateTopics.

@frairon
Copy link
Contributor

frairon commented May 17, 2021

Heyhey, so you're right it was kind of a breaking change, but only if we consider an earlier bug a "feature" :).
The auto-topic creation was actually not intended and was a misbehavior, if the cluster was configured to auto-create topics. On clusters with auto-create it would not create the topics. Also this behavior led to misconfigured topics. E.g. when creating Views before creating the processor, the processor-table ended up configured with cleanup-policy "retention" instead of "compact" which made kafka lose the state eventually.

So instead of reverting the original behavior I'd also suggest creating the topics by default and allowing a config option to disable that behavior. Processors and Views use TopicManagers internally already, so we can add EnsureStream/TableExists in suitable places. However we have to derive some kind of "default"-partition count. A processor e.g. creates the partition-table with the same number of partitions as the input topics. If nothing is created yet, it needs to know which number of partitions to fall back to. But apart from that it shouldn't be hard.
@mrahs Would you like to give this a try?

@mrahs
Copy link
Contributor Author

mrahs commented May 17, 2021

Heyhey, so you're right it was kind of a breaking change, but only if we consider an earlier bug a "feature" :).

😀

Only our daily clusters are configured with auto-create, so I didn't give it a lot of thoughts for production use. You bring up good points about default topic configurations. I'd rather that stayed out of scope for goka since it should already be configured in Kafka. Otherwise, topic creation should be properly implemented in the topic manager.

My personal bias is to remain predictable. If I've configured my cluster to auto-create topics, I wouldn't expect apps (goka or otherwise) to fail because of missing topics.

Thoughts?

@frairon
Copy link
Contributor

frairon commented May 19, 2021

Ok I agree that topic creation should mostly be done externally. However creating the table-topic and loop-topics should be done in goka, because the environment cannot know for sure how to name/configure them. And as said above, letting the cluster auto-create those topics in particular, will lead to errors in production if the cluster is misconfigured. Maybe we can think of a hybrid solution?

  • for topics that goka is responsible for (tables for views, processors and loop-topics), we always create them
  • for topics we would assume exist in production, but need them in testing, we have a (by default enabled option) of letting the cluster auto-create them. Maybe it's possible by using this with empty arguments so the cluster uses it's configured efault settings (will have to investigate that). That should be semantically the same as the earlier bug that made the cluster create the topics.

What do you think of that?

@mrahs
Copy link
Contributor Author

mrahs commented May 25, 2021

Sorry for the delay. I'm busy with life atm until the second week of June. I'll try to get back to this issue as soon as I can.

@mrahs
Copy link
Contributor Author

mrahs commented Jul 30, 2021

I like the hybrid approach. I think the current approach of not specifying a topic to avoid creating it makes sense. So, for goka's owned topics, we check if they exist without triggering auto-create, and create them if they do not exist. For user defined topics, we just check if they exist and let the broker decide if it should auto-create or fail. In other words, goka shouldn't introduce a duplicate configuration value for topic auto-creation and should rather delegate to the broker. Thoughts?

@mrahs mrahs changed the title Breaking change in topic creation behvaiour Breaking change in topic auto creation behvaiour Jul 30, 2021
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

2 participants