We want to make a small additional change and piggy-back it to this KIP. To exploit the new feature of AdmintClient in KafkaStreams, we want to update the default value of Streams configuration parameter `replication.factor` from `1` to `-1`.
This config change will of course only go into 2.4 release. I updated the KIP accordingly and created https://issues.apache.org/jira/browse/KAFKA-8531 Let us know if there are any concerns. I don't think that we need to revote base on this tiny change. -Matthias On 5/28/19 8:50 AM, Almog Gavra wrote: > Hello everyone - the PR is out and ready to review! > https://github.com/apache/kafka/pull/6728/ > > On Fri, May 10, 2019 at 11:57 AM Almog Gavra <al...@confluent.io> wrote: > >> Thanks everyone for the comments and discussion! Closing the voting out >> for this KIP: >> >> * 4 Binding (Randall, Manikumar, Colin, Gwen) >> * 2 Non-Binding (Ryanne, Mickael) >> >> Cheers, >> Almog >> >> >> On Fri, May 10, 2019 at 11:55 AM Gwen Shapira <g...@confluent.io> wrote: >> >>> +1 (binding) >>> >>> On Fri, May 10, 2019 at 9:32 AM Almog Gavra <al...@confluent.io> wrote: >>> >>>> I'm happy pulling it out into a separate KIP to target the discussion. >>> This >>>> one can just introduce the "default" constructor for no partitions or >>>> replicas since we'll need that one whether or not we add the builder. >>>> >>>> Updated the KIP moving the builder to a section in "Rejected >>> Alternatives - >>>> Follow Up KIP" - @Randall since your +1 was for the KIP with builder I >>> want >>>> to make sure that is okay with you. >>>> >>>> On Fri, May 10, 2019 at 9:14 AM Colin McCabe <cmcc...@apache.org> >>> wrote: >>>> >>>>> Given that there are still some open questions about the builder, >>> maybe >>>> we >>>>> should put it in a separate KIP? >>>>> >>>>> best, >>>>> Colin >>>>> >>>>> >>>>> On Fri, May 10, 2019, at 09:00, Ryanne Dolan wrote: >>>>>> +1 (non-binding) for the core feature, but I could take or leave the >>>>>> builder. >>>>>> >>>>>> Ryanne >>>>>> >>>>>> On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io> >>>> wrote: >>>>>> >>>>>>> @Ismael - I agree that the methods are a little random. They were >>>> just >>>>>>> ported from what's currently in the connect builder. I think a >>> better >>>>>>> option might be to keep the connect builder around and have extend >>>> from >>>>>>> this builder, and make this builder only implement the "critical" >>>>> methods >>>>>>> (e.g. replicas/partitions/config). (cc Randall) >>>>>>> >>>>>>> Is passing optionals in the constructor something that's common in >>>> AK? >>>>> I >>>>>>> think my preference would be toward the builder so that it's >>> easier >>>> to >>>>>>> extend as Randall mentioned. >>>>>>> >>>>>>> Almog >>>>>>> >>>>>>> On Fri, May 10, 2019 at 8:17 AM Ismael Juma <ism...@juma.me.uk> >>>> wrote: >>>>>>> >>>>>>>> The current builder includes random methods like >>>>> uncleanLeaderElection. >>>>>>>> That doesn't make sense to me since it's a topic config (and we >>>> don't >>>>>>>> include methods for other topic configs). Also, I'm not sure >>> about >>>>> the >>>>>>>> naming convention, should we have a `with` prefix? It would be >>> good >>>>> to >>>>>>>> check existing builders in `clients` if any exist for what >>> they're >>>>> doing. >>>>>>>> >>>>>>>> If we didn't add a builder, another option would be a single new >>>>>>>> constructor: >>>>>>>> >>>>>>>> public NewTopic(String name, Optional<Integer> numPartitions, >>>>>>>> Optional<Short> replicationFactor) >>>>>>>> >>>>>>>> Ismael >>>>>>>> >>>>>>>> On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io> >>>>> wrote: >>>>>>>> >>>>>>>>> Thanks Colin! Since the discussion around the builder is here >>>> I'll >>>>> copy >>>>>>>>> over my comment from the discuss thread: >>>>>>>>> >>>>>>>>> If we want the flexibility that the builder provides we would >>>> need >>>>> to >>>>>>> add >>>>>>>>> three constructors: >>>>>>>>> - no partitions/replicas >>>>>>>>> - just partitions >>>>>>>>> - just replicas >>>>>>>>> >>>>>>>>> I see good use cases for the first two - the third (just >>>> replicas) >>>>>>> seems >>>>>>>>> less necessary but complicates the API a bit (you have to >>>>> differentiate >>>>>>>>> NewTopic(int) with NewTopic(short) or something like that). If >>>>> we're >>>>>>>> happy >>>>>>>>> with a KIP that covers just the first two then I can remove >>> the >>>>> builder >>>>>>>> to >>>>>>>>> simplify things. Otherwise, I think the builder is an >>> important >>>>>>> addition. >>>>>>>>> >>>>>>>>> Thoughts? >>>>>>>>> >>>>>>>>> On Thu, May 9, 2019 at 2:50 PM Colin McCabe < >>> cmcc...@apache.org> >>>>>>> wrote: >>>>>>>>> >>>>>>>>>> +1 (binding). >>>>>>>>>> >>>>>>>>>> Re: the builder discussion. I don't feel strongly either >>> way-- >>>>> the >>>>>>>>>> builder sketched out in the KIP looks reasonable, but I can >>>> also >>>>>>>>> understand >>>>>>>>>> Ismael's argument for keeping the KIP minimal. >>>>>>>>>> >>>>>>>>>> best, >>>>>>>>>> Colin >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Thu, May 9, 2019, at 08:09, Randall Hauch wrote: >>>>>>>>>>> I'm fine with simplifying the KIP by removing the Builder >>>>> (which >>>>>>>> seems >>>>>>>>>>> ancillary), or keeping the KIP as-is. I'll wait to vote >>> until >>>>> Almog >>>>>>>>> says >>>>>>>>>>> which way he'd like to proceed. >>>>>>>>>>> >>>>>>>>>>> On Thu, May 9, 2019 at 9:45 AM Ismael Juma < >>>> ism...@juma.me.uk> >>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Almog, >>>>>>>>>>>> >>>>>>>>>>>> Adding a Builder seems unrelated to this change. Do we >>> need >>>>> it? >>>>>>>> Given >>>>>>>>>> the >>>>>>>>>>>> imminent KIP deadline, I'd keep it simple and just have >>> the >>>>>>>>> constructor >>>>>>>>>>>> with just the name parameter. >>>>>>>>>>>> >>>>>>>>>>>> Ismael >>>>>>>>>>>> >>>>>>>>>>>> On Thu, May 2, 2019 at 1:58 AM Mickael Maison < >>>>>>>>>> mickael.mai...@gmail.com> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> I was planning to write a KIP for the exact same >>> feature! >>>>>>>>>>>>> +1 (non binding) >>>>>>>>>>>>> >>>>>>>>>>>>> Thanks for the KIP >>>>>>>>>>>>> >>>>>>>>>>>>> On Wed, May 1, 2019 at 7:24 PM Almog Gavra < >>>>> al...@confluent.io >>>>>>>> >>>>>>>>>> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>> Hello Everyone! >>>>>>>>>>>>>> >>>>>>>>>>>>>> Kicking off the voting for >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>> >>>> >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> You can see discussion thread here (please respond >>> with >>>>>>>>>> suggestions on >>>>>>>>>>>>> that >>>>>>>>>>>>>> thread): >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>> >>>> >>> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>> Almog >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >>> >>> -- >>> *Gwen Shapira* >>> Product Manager | Confluent >>> 650.450.2760 | @gwenshap >>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog >>> <http://www.confluent.io/blog> >>> >> >
signature.asc
Description: OpenPGP digital signature
