Thank you Federico for answering the questions. No more questions/concerns from me. The KIP looks good.
-- Divij Vaidya On Wed, Jun 26, 2024 at 11:02 AM Federico Valeri <fedeval...@gmail.com> wrote: > Hi Divij, thanks for you questions and suggestions, much appreciated. > > On Tue, Jun 25, 2024 at 1:12 PM Divij Vaidya <divijvaidy...@gmail.com> > wrote: > > > > Hey all > > > > Seems like we have quite a lot of interest in adding this dedicated flag > in > > addition to existing alternatives. In that case, I will cede to the > > majority interest here. Let's go ahead with this new flag. > > > > TBH, I like your idea of moving away from dedicated decode flags, but > I think we would need a major tool refactoring to make it user > friendly. Maybe using the Java service loader mechanism we can avoid > having to specify the Decoder's FQCN. That said, I would leave this as > a possible future KIP, but in the meantime we can have the simple > solution described in the KIP. > > > I have one last question about the new flag: > > > > Can you please help me understand if the tool knows which schema version > > (apiKey in the schema defined in RemoteLogSegmentMetadataRecord.json) to > > use while decoding a record? > > The RemoteMetadataLogMessageParser will use the > RemoteLogMetadataSerde.deserialize method, which is able to extract > apiKey and version from the record's payload, and select the correct > RemoteLogMetadataTransform instance required for decoding. > > Could you also please update the KIP (and the > > manual) mentioning whether the tool works with segments containing mixed > > versions of records or do we need to specify a particular schema version? > > > > Currently all RemoteLogSegmentMetadata schemas are at version 0, so we > cannot have mixed versions of records. In the future, these schemas > may evolve, so the RemoteLogMetadataSerde will need to be updated to > decode all supported versions. KIP updated. > > > > > > -- > > Divij Vaidya > > > > > > > > On Thu, Jun 20, 2024 at 5:57 PM Kamal Chandraprakash < > > kamal.chandraprak...@gmail.com> wrote: > > > > > Hi Federico, > > > > > > Thanks for the KIP! +1 from me. > > > > > > On Wed, Jun 19, 2024, 17:36 Luke Chen <show...@gmail.com> wrote: > > > > > > > Hi Federico, > > > > > > > > Thanks for the KIP! > > > > It's helpful for debugging the tiered storage issues. > > > > +1 from me. > > > > > > > > Thanks. > > > > Luke > > > > > > > > On Tue, Jun 18, 2024 at 12:18 AM Satish Duggana < > > > satish.dugg...@gmail.com> > > > > wrote: > > > > > > > > > Thanks Federico for the KIP. > > > > > > > > > > This feature is helpful for developers while debugging tiered > storage > > > > > related issues. > > > > > > > > > > Even though RLMM is a pluggable interface, it is still useful to > have > > > > > a utility that is meant for the default/inbuilt implementation > based > > > > > on the internal topic. We can clarify that in the help notes and > user > > > > > docs. > > > > > > > > > > Users can still use alternatives like others suggested if they > need to > > > > > dump in a different format > > > > > - Running the dump-logs tool with custom decoder > > > > > - Running kafka-consumer.sh on the topic. > > > > > > > > > > ~Satish. > > > > > > > > > > > > > > > ~Satish. > > > > > > > > > > > > > > > > > > > > On Mon, 17 Jun 2024 at 15:55, Federico Valeri < > fedeval...@gmail.com> > > > > > wrote: > > > > > > > > > > > > Hi Kamal, > > > > > > > > > > > > On Mon, Jun 17, 2024 at 11:44 AM Kamal Chandraprakash > > > > > > <kamal.chandraprak...@gmail.com> wrote: > > > > > > > > > > > > > > We can use the console-consumer to read the contents of the > > > > > > > `__remote_log_metadata` topic. Why are we proposing a new tool? > > > > > > > > > > > > > > sh kafka-console-consumer.sh --bootstrap-server localhost:9092 > > > > --topic > > > > > > > __remote_log_metadata --consumer-property > > > > > exclude.internal.topics=false > > > > > > > --formatter > > > > > > > > > > > > > > > > > > > > org.apache.kafka.server.log.remote.metadata.storage.serialization.RemoteLogMetadataSerde\$RemoteLogMetadataFormatter > > > > > > > --from-beginning > > > > > > > > > > > > > > > > > > > Thanks from bringing this up. It works fine but a running broker > is > > > > > > required, so it would make it inconvenient for a remote support > > > > > > engineer. Also you may have to deal with client security > > > > > > configuration, and it would be complicated to only dump specific > > > > > > segments. I'm adding to the rejected alternative for now, but I'm > > > open > > > > > > to changes. > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Jun 17, 2024 at 12:53 PM Federico Valeri < > > > > fedeval...@gmail.com > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > Hi Divij, > > > > > > > > > > > > > > > > On Sun, Jun 16, 2024 at 7:38 PM Divij Vaidya < > > > > > divijvaidy...@gmail.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hello Federico > > > > > > > > > > > > > > > > > > Please note that the topic-based RLMM is one of the > possible > > > > > > > > > implementations of RLMM. Hence, whatever solution we design > > > here > > > > > should: > > > > > > > > 1\ > > > > > > > > > be explicit that this tooling only works for topic based > RLMM > > > 2\ > > > > > specify > > > > > > > > > the handling of the failure mode when topic based RLMM is > not > > > > > being used. > > > > > > > > > > > > > > > > > > > > > > > > > That's true, thanks for pointing out. > > > > > > > > > > > > > > > > > I would argue that Topic based RLMM cannot be treated the > same > > > as > > > > > other > > > > > > > > > internal topics. Topic based RLMM topic is an optional > topic > > > > which > > > > > can > > > > > > > > have > > > > > > > > > any possible schema (depending on plugin implementation) > > > whereas > > > > > > > > > other internal topics are always guaranteed to be present > with > > > a > > > > > fixed > > > > > > > > > schema. > > > > > > > > > > > > > > > > > > > > > > > > > Right, I updated the KIP with an improved option description. > > > > > > > > > > > > > > > > > In light of the above statements, the rejected alternative > > > sounds > > > > > better > > > > > > > > to > > > > > > > > > me because: > > > > > > > > > 1\ it provides the ability to dump logs for "any" RLMM > > > > > implementation and > > > > > > > > > not just topic based RLMM. > > > > > > > > > 2\ we don't have to deal with schema evolution of topic > based > > > > RLMM > > > > > in > > > > > > > > this > > > > > > > > > tool. That responsibility will be delegated to the decoder > > > class > > > > > which > > > > > > > > the > > > > > > > > > operator can define using the flag "--value-decoder-class". > > > > > > > > > > > > > > > > > > Is there a reason that you are unable to use the rejected > > > > solution > > > > > (which > > > > > > > > > requires no changes) for debugging purposes? > > > > > > > > > > > > > > > > > > > > > > > > > The rejected alternative will still be available, but I > thought > > > > that > > > > > > > > having a dedicated flag would make debugging easier, as I > guess > > > > most > > > > > > > > people will use the default RLMM implementation. I would be > happy > > > > to > > > > > > > > hear other opinions on this. > > > > > > > > > > > > > > > > > -- > > > > > > > > > Divij Vaidya > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Jun 15, 2024 at 4:43 PM Federico Valeri < > > > > > fedeval...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > I'd like to kick off a discussion for KIP-1057, that > proposes > > > > to > > > > > add > > > > > > > > > > remote log metadata flag to the dump log tool, which is > > > useful > > > > > when > > > > > > > > > > debugging. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1057%3A+Add+remote+log+metadata+flag+to+the+dump+log+tool > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > Fede > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
