Hi Vedarth, Thanks for the quick reply and for the clarifications.
I agree with your analysis and the alternative approach you found makes sense. I'll try to take a look at your PR later on today or tomorrow. Thanks, Mickael On Wed, Dec 20, 2023 at 1:33 PM Vedarth Sharma <vedarth.sha...@gmail.com> wrote: > > Hi Mickael, > > In our previous approach startup time would have been affected if the > utility script code was written in some other language. If we had just > replaced the utility script with java code that would have impacted startup > time significantly. In my local machine, it took around 50ms (on M2 silicon > chip) just for a jvm process to start and close. Doing this around 10 times > (number of times we were calling the utility script) would have caused > performance issues, especially on slower machines. > > I agree that the decision about which approach to take should be made by > committers. When you raised the issue I re-looked at the existing solution > and found that if I move all the logic in the wrapper, I will be able to > achieve the same performance without the need of any utility code like the > golang code I had. And this wrapper code will only be executed once during > the docker container startup instead of multiple times as we were doing > with utility script written in golang. > > So the problem that this approach is solving is not just removing the > golang code, but also removing the need of any utility script. > > This is just an approach that I feel is an improvement over current golang > solution, but I would be happy to take community's feedback and opinion on > this and we can go with what committers feels is the better solution. > > Thanks and regards, > Vedarth > > On Wed, Dec 20, 2023 at 5:05 PM Mickael Maison <mickael.mai...@gmail.com> > wrote: > > > Hi Vedarth, > > > > In the PR when I raised concerns about introducing Go to the > > repository you responded "Startup time will get impacted though, as > > java is significantly slower than golang and we are calling this > > script multiple times.". It would be good to provide numbers if you > > have, so the community can make a decision. Maybe we have multiple > > experts with Go and the risk of introducing this language is reduced, > > but because this was not discussed, we don't know. > > > > When you say, "we have opted to take a different approach", who is > > "we"? I think this decision should be made by the committers. > > > > I marked the Jira (https://issues.apache.org/jira/browse/KAFKA-16016) > > as a blocker for 3.7 as I think we need to make this decision before > > releasing the docker images. > > > > Thanks, > > Mickael > > > > On Wed, Dec 20, 2023 at 10:16 AM Vedarth Sharma > > <vedarth.sha...@gmail.com> wrote: > > > > > > Hello everyone, > > > > > > In the course of implementing this KIP, we introduced a small piece of > > code > > > in golang to prepare property files for the Docker image. Our decision > > was > > > influenced by considerations such as performance, code testability, and a > > > reduction in the final Docker image size, as outlined in > > > https://github.com/apache/kafka/pull/14552#issuecomment-1855353838. > > > However, upon reflection, we acknowledge that incorporating golang > > > introduces a new language into the repository, potentially leading to > > > increased maintenance overhead. In light of this, we have opted to take a > > > different approach by introducing a Docker wrapper within the Kafka > > > codebase. This wrapper will remove the need for the Golang code. > > > To streamline this adjustment, we have submitted a PR: > > > https://github.com/apache/kafka/pull/15048. The proposed changes involve > > > the removal of the previously added golang code. > > > This change has also been documented in the KIP. > > > > > > Thanks and regards, > > > Vedarth > > > > > > On Thu, Oct 26, 2023 at 8:09 AM Ismael Juma <m...@ismaeljuma.com> wrote: > > > > > > > Hi Vedarth, > > > > > > > > > > > > > Local Kafka startup time (without JSA): 1.592 secs > > > > > Local Kafka startup time (with JSA): 1.016 secs > > > > > Local Kafka startup memory usage (without JSA): 440MB > > > > > Local Kafka startup memory usage (with JSA): 380MB > > > > > > > > > > > > This is a significant reduction in start-up time (33%) - nice! > > > > > > > > Ismael > > > > > > > > On Wed, Oct 25, 2023 at 10:24 AM Vedarth Sharma < > > vedarth.sha...@gmail.com> > > > > wrote: > > > > > > > > > Hi Ismael! > > > > > > > > > > Thanks for bringing this to our attention. > > > > > > > > > > We did a small POC integrating CDS with Kafka server startup, and > > > > > encountered positive outcomes(results are added in the KIP). > > > > > Hence, we've decided to include the dynamically generated JSA file > > from > > > > the > > > > > following workflow in the Docker image: > > > > > > > > > > 1. Start Kafka > > > > > 2. Create a topic > > > > > 3. Produce messages > > > > > 4. Consume messages > > > > > 5. Stop Kafka > > > > > > > > > > Additionally, we've identified some limitations of CDS, which have > > also > > > > > been detailed in the KIP. > > > > > > > > > > Thanks and regards, > > > > > Vedarth > > > > > > > > > > On Wed, Oct 25, 2023 at 10:56 AM Ismael Juma <m...@ismaeljuma.com> > > wrote: > > > > > > > > > > > The reference I meant to include: > > > > > > > > > > > > > > https://docs.oracle.com/en/java/javase/17/vm/class-data-sharing.html > > > > > > > > > > > > On Tue, Oct 24, 2023, 10:25 PM Ismael Juma <m...@ismaeljuma.com> > > wrote: > > > > > > > > > > > > > Hi Krishna, > > > > > > > > > > > > > > One last question from me, did we confuse using AppCDS or Dynamic > > > > CDS? > > > > > > > > > > > > > > Thanks, > > > > > > > Ismael > > > > > > > > > > > > > > On Tue, Oct 24, 2023, 9:54 PM Krishna Agarwal < > > > > > > > krishna0608agar...@gmail.com> wrote: > > > > > > > > > > > > > >> Hi, > > > > > > >> Thanks for the insightful feedback on this KIP. If there are no > > > > > further > > > > > > >> questions, I'm considering wrapping up this discussion thread. > > We'll > > > > > be > > > > > > >> moving into the voting process in the next couple of days. Your > > > > > > continued > > > > > > >> input is greatly appreciated! > > > > > > >> > > > > > > >> Regards, > > > > > > >> Krishna > > > > > > >> > > > > > > >> On Fri, Sep 8, 2023 at 1:27 PM Krishna Agarwal < > > > > > > >> krishna0608agar...@gmail.com> > > > > > > >> wrote: > > > > > > >> > > > > > > >> > Hi, > > > > > > >> > Apache Kafka does not have an official docker image currently. > > > > > > >> > I want to submit a KIP to publish a docker image for Apache > > Kafka. > > > > > > >> > > > > > > > >> > KIP-975: Docker Image for Apache Kafka > > > > > > >> > < > > > > > > >> > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-975%3A+Docker+Image+for+Apache+Kafka > > > > > > >> > > > > > > > >> > > > > > > > >> > Regards, > > > > > > >> > Krishna > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >
