Heya everyone! re: Doguscan
I believe the answer to 101 needs a bit more discussion. As far as I know, tiered storage today has methods to update a metadata of a segment to say "hey, I would like this deleted", but actual deletion is left to plugin implementations (or any background cleaners). In other words, there is no "immediate" deletion. In this KIP, we would like to continue doing the same if the retention policy is set to delete. So I believe the answer is actually that a) we will update the metadata of the segments to mark them as deleted and b) we will advance the log start offset. Any deletion of actual files will still be delegated to plugin implementations. I believe this is further supported by "*remote.log.disable.policy=delete:* Logs that are archived in the remote storage will not be part of the contiguous "active" log and will be deleted asynchronously as part of the disablement process" Following from the above, I believe for 102 it is fine to allow setting of remote.log.disable.policy on a disabled topic in much the same way we allow other remote-related configurations to be set on a topic (i.e. local.retention.*) - it just won't have an effect. Granted, I do believe we should restrict the policy being changed while a disablement is ongoing. re: Satish and Kamal 104, 1 and 2 are fair asks, I will work with Doguscan to update the KIP with the information! Best, Christo On Thu, 28 Mar 2024 at 10:31, Doğuşcan Namal <namal.dogus...@gmail.com> wrote: > Hi Satish, I will try to answer as much as I can and the others could chime > in with further details. > > > > > > *101. For remote.log.disable.policy=delete: Does it delete the remote log > data immediately and the data in remote storage will not be taken into > account by any replica? That means log-start-offset is moved to the earlier > local-log-start-offset.* > *Exactly. RemoteLogData will be deleted immediately. * > *So before the deletion starts we move LogStart offset to > LocalLogStartOffset to ensure that no RemoteLog will be accessed after that > point.* > > > * 102. Can we update the remote.log.disable.policy after tiered storage is > disabled on a topic?* > > *This is a good point. I think we should not allow modifying this > configuration* > *because changing the policy from Deletion to Retain when there is an > ongoing Deletion will result in an undefined behaviour and where we retain > half of the remote log and delete the other half.* > > * 103. Do we plan to add any metrics related to this feature?* > > > > *Any recommendations?* > *We may emit a gauge showing the enablement state of a topic but we could > gather that info from the logs as well.* > *The total duration for remote topic deletion could be added as well but > this is more of a metric for the RemotePartitionRemover itself.* > > > > *104. Please add configuration details about copier thread pool, expiration > thread pool and the migration of the existing > RemoteLogManagerScheduledThreadPool.* > > *Will add the details.* > > 105. How is the behaviour with topic or partition deletion request > handled when tiered storage disablement request is still being > processed on a topic? > > *If the disablement policy is Delete then a successive topic deletion > request is going to be a NOOP because RemoteLogs is already being deleted.* > *If the disablement policy is Retain, then we only moved the LogStartOffset > and didn't touch RemoteLogs anyway, so the delete topic request will > result* > > *in the initiation of RemoteLog deletion.* > > > On Tue, 26 Mar 2024 at 18:21, Kamal Chandraprakash < > kamal.chandraprak...@gmail.com> wrote: > > > Hi, > > > > Thanks for the KIP! Overall the KIP looks good and covered most of the > > items. > > > > 1. Could you explain how the brokers will handle the DisableRemoteTopic > API > > request? > > > > 2. Who will initiate the controller interaction sequence? Does the > > controller listens for > > topic config updates and initiate the disablement? > > > > -- > > Kamal > > > > On Tue, Mar 26, 2024 at 4:40 PM Satish Duggana <satish.dugg...@gmail.com > > > > wrote: > > > > > Thanks Mehari, Divij, Christo etal for the KIP. > > > > > > I had an initial review of the KIP and left the below comments. > > > > > > 101. For remote.log.disable.policy=delete: > > > Does it delete the remote log data immediately and the data in remote > > > storage will not be taken into account by any replica? That means > > > log-start-offset is moved to the earlier local-log-start-offset. > > > > > > 102. Can we update the remote.log.disable.policy after tiered storage > > > is disabled on a topic? > > > > > > 103. Do we plan to add any metrics related to this feature? > > > > > > 104. Please add configuration details about copier thread pool, > > > expiration thread pool and the migration of the existing > > > RemoteLogManagerScheduledThreadPool. > > > > > > 105. How is the behaviour with topic or partition deletion request > > > handled when tiered storage disablement request is still being > > > processed on a topic? > > > > > > ~Satish. > > > > > > On Mon, 25 Mar 2024 at 13:34, Doğuşcan Namal <namal.dogus...@gmail.com > > > > > wrote: > > > > > > > > Hi Christo and Luke, > > > > > > > > I think the KRaft section of the KIP requires slight improvement. The > > > metadata propagation in KRaft is handled by the RAFT layer instead of > > > sending Controller -> Broker RPCs. In fact, KIP-631 deprecated these > > RPCs. > > > > > > > > I will come up with some recommendations on how we could improve that > > > one but until then, @Luke please feel free to review the KIP. > > > > > > > > @Satish, if we want this to make it to Kafka 3.8 I believe we need to > > > aim to get the KIP approved in the following weeks otherwise it will > slip > > > and we can not support it in Zookeeper mode. > > > > > > > > I also would like to better understand what is the community's stand > > for > > > adding a new feature for Zookeeper since it is marked as deprecated > > already. > > > > > > > > Thanks. > > > > > > > > > > > > > > > > On Mon, 18 Mar 2024 at 13:42, Christo Lolov <christolo...@gmail.com> > > > wrote: > > > >> > > > >> Heya, > > > >> > > > >> I do have some time to put into this, but to be honest I am still > > after > > > >> reviews of the KIP itself :) > > > >> > > > >> After the latest changes it ought to be detailing both a Zookeeper > > > approach > > > >> and a KRaft approach. > > > >> > > > >> Do you have any thoughts on how it could be improved or should I > > start a > > > >> voting thread? > > > >> > > > >> Best, > > > >> Christo > > > >> > > > >> On Thu, 14 Mar 2024 at 06:12, Luke Chen <show...@gmail.com> wrote: > > > >> > > > >> > Hi Christo, > > > >> > > > > >> > Any update with this KIP? > > > >> > If you don't have time to complete it, I can collaborate with you > to > > > work > > > >> > on it. > > > >> > > > > >> > Thanks. > > > >> > Luke > > > >> > > > > >> > On Wed, Jan 17, 2024 at 11:38 PM Satish Duggana < > > > satish.dugg...@gmail.com> > > > >> > wrote: > > > >> > > > > >> > > Hi Christo, > > > >> > > Thanks for volunteering to contribute to the KIP discussion. I > > > suggest > > > >> > > considering this KIP for both ZK and KRaft as it will be helpful > > for > > > >> > > this feature to be available in 3.8.0 running with ZK clusters. > > > >> > > > > > >> > > Thanks, > > > >> > > Satish. > > > >> > > > > > >> > > On Wed, 17 Jan 2024 at 19:04, Christo Lolov < > > christolo...@gmail.com > > > > > > > >> > > wrote: > > > >> > > > > > > >> > > > Hello! > > > >> > > > > > > >> > > > I volunteer to get this KIP moving forward and implemented in > > > Apache > > > >> > > Kafka > > > >> > > > 3.8. > > > >> > > > > > > >> > > > I have caught up with Mehari offline and we have agreed that > > given > > > >> > Apache > > > >> > > > Kafka 4.0 being around the corner we would like to propose > this > > > feature > > > >> > > > only for KRaft clusters. > > > >> > > > > > > >> > > > Any and all reviews and comments are welcome! > > > >> > > > > > > >> > > > Best, > > > >> > > > Christo > > > >> > > > > > > >> > > > On Tue, 9 Jan 2024 at 09:44, Doğuşcan Namal < > > > namal.dogus...@gmail.com> > > > >> > > > wrote: > > > >> > > > > > > >> > > > > Hi everyone, any progress on the status of this KIP? Overall > > > looks > > > >> > > good to > > > >> > > > > me but I wonder whether we still need to support it for > > > Zookeeper > > > >> > mode > > > >> > > > > given that it will be deprecated in the next 3 months. > > > >> > > > > > > > >> > > > > On 2023/07/21 20:16:46 "Beyene, Mehari" wrote: > > > >> > > > > > Hi everyone, > > > >> > > > > > I would like to start a discussion on KIP-950: Tiered > > Storage > > > >> > > Disablement > > > >> > > > > ( > > > >> > > > > > > > >> > > > > > > > >> > > > > > >> > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-950%3A++Tiered+Storage+Disablement > > > >> > > > > ). > > > >> > > > > > > > > >> > > > > > This KIP proposes adding the ability to disable and > > re-enable > > > >> > tiered > > > >> > > > > storage on a topic. > > > >> > > > > > > > > >> > > > > > Thanks, > > > >> > > > > > Mehari > > > >> > > > > > > > > >> > > > > > > > >> > > > > > >> > > > > > > >
