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 > > > > > > > > > > > > > > > > > > > > > > > > >
