Deduplicate topic declaration before trying to create them #383
Conversation
| public bool Equals(TopicConfiguration x, TopicConfiguration y) => x?.Name == y?.Name; | ||
|
|
||
| public int GetHashCode(TopicConfiguration obj) => obj.Name.GetHashCode(); | ||
| } |
There was a problem hiding this comment.
Maybe throwing an exception should be better. What do you think?
There was a problem hiding this comment.
It's whats happening now. The issue with the exception is that it means that we must only call createtopics one time per topic.
In our case for a topic we have many handler and by default we call create topic. The registration happen in the bounded context of the handler and is then use in the app. Each bounded context are not aware of how the registration has been made for other bounded context.
There was a problem hiding this comment.
Since KafkaFlow doesn't throw an exception if it already exists, I think it makes sense not to throw it in case of duplicated configuration. I believe that as a consumer, I would expect the first attempt to succeed and the second to be ignored.
There was a problem hiding this comment.
Hi @kYann
We discussed this issue and @filipeesch raised a scenario that we are concerned with.
If eventually, we have a duplicate topic declaration with different parameters (number of partitions and replicas), a silent deduplication process may induce confusion.
So, we believe that at least we should log a warning mentioning the duplication and that we are deduplicating and using the settings X and Y to try to create the topic.
What do you think?
There was a problem hiding this comment.
Good point. I think the warning is a good idea. There is no other way to resolve this.
If the warning could happen only when the parameters are different would be a nice touch.
| public bool Equals(TopicConfiguration x, TopicConfiguration y) => x?.Name == y?.Name; | ||
|
|
||
| public int GetHashCode(TopicConfiguration obj) => obj.Name.GetHashCode(); | ||
| } |
There was a problem hiding this comment.
Hi @kYann
We discussed this issue and @filipeesch raised a scenario that we are concerned with.
If eventually, we have a duplicate topic declaration with different parameters (number of partitions and replicas), a silent deduplication process may induce confusion.
So, we believe that at least we should log a warning mentioning the duplication and that we are deduplicating and using the settings X and Y to try to create the topic.
What do you think?
|
Sorry I thought I answered but did not. Yes I agree with your solution. |
@kYann are you available to review the PR applying that? |
|
Yes ! |
|
Hello, any updates on this? Seems like everything is ready, only conflicting megre is left. |
Description
Deduplicate topic list before sending it to AdminClient. Handle case where client call multiple CreateTopicIfNotExists with same topic name.
Fixes #382
How Has This Been Tested?
Tested on our solution
Checklist
Disclaimer
By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement