Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi Satish, Thanks for the review. I've updated the KIP to move ZK based solution to the appendix section. > One minor comment I have is to change the config name from "remote.log.copy.disable" to "remote.log.copy.enable" with the default value being true. I also think the original name "remote.l

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi Satish, Thanks for the review! > One minor comment I have is to change the config name from "remote.log.copy.disable" to "remote.log.copy.enable" with the default value being true. I'm inclined towards maintaining the config as "remote.log.copy.disable" to keep the default value as "false".

Re: [VOTE] KIP-950: Tiered Storage Disablement

Thanks Kamal, and Luke for improving the earlier solution for KRaft. One minor comment I have is to change the config name from "remote.log.copy.disable" to "remote.log.copy.enable" with the default value being true. The solution summary to disable tiered storage on a topic: - When a user wants

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi all, Based on the original design: When tiered storage is disabled or becomes read-only on a topic, the local retention configuration becomes irrelevant, and all data expiration follows the topic-wide retention configuration exclusively. That works well. But we are afraid users will not check

Re: [VOTE] KIP-950: Tiered Storage Disablement

Thanks Kamal for the comments. KIP updated. Thanks. Luke On Fri, Jul 26, 2024 at 6:56 PM Kamal Chandraprakash < kamal.chandraprak...@gmail.com> wrote: > Luke, > > Thanks for confirming the topic config change validation on the controller > and updating the KIP. > The updated KIP LGTM. > > 1. Can

Re: [VOTE] KIP-950: Tiered Storage Disablement

Luke, Thanks for confirming the topic config change validation on the controller and updating the KIP. The updated KIP LGTM. 1. Can we update the below sentence in the KIP to clarify that remote.storage.enable should be true during graceful disablement? > Users set the configuration "remote.stor

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi Kamal, Thanks for the comments. For this: > If we throw an exception from the server for invalid config, then there will be inconsistency between the CLI tools and the actual state of the topic in the cluster. This can cause some confusion to the users whether tiered storage is disabled or not

Re: [VOTE] KIP-950: Tiered Storage Disablement

Correction: (2): Wait for all the remote segments to be deleted async due to breach by retention time (or) size, then set the `remote.storage.enable = false` and `remote.log.delete.on.disable = true`. This step is optional. On Thu, Jul 25, 2024 at 11:13 PM Kamal Chandraprakash < kamal.chan

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi Chia-Ping, Thanks for the review! > If so, what is the purpose of `remote.log.delete.on.disable=false`? IIUC, the purpose of `remote.log.delete.on.disable` is to get explicit confirmation from the user before deleting the remote log segments. The concern raised in the thread is that if the u

Re: [VOTE] KIP-950: Tiered Storage Disablement

remote.storage.enable=false remote.log.delete.on.disable=false (default) If the topic config is set to this, or changed to this, we'll return ConfigException during validation. Pardon me, I'm a bit confused. when `remote.storage.enable=true`, `remote.log.delete.on.disable=false` is no-op when `re

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi Christo, Thanks for your reply. > keep the remote.log.disable.policy, but only allow it to take a value of "delete". I agree, or maybe make it a boolean value, and rename it to `remote.log.delete.on.disable`, which is clearer. And because of this new config, there will be a case that the conf

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hello! Thank you for raising this! Up to now KIP-950 took the stance that you can disable tiering whenever you wish as long as you specify what you would like to do with the data in remote. Amongst other things it also made the promise that it will not delete data without a user explicitly saying

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi all, I just found the table is not able to be displayed correctly in the email. I've put the table content in google doc here . Thanks. Luke On Thu, Jul 25, 2024 at 6:30 PM Luke Chen wrote: >

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi all, While implementing the feature in KRaft mode, I found something we need to change the original proposal: (1) In the KIP of "Disablement - KRaft backed Cluster

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi all, Thanks for the KIP. This is definitely a worthwhile feature. However, I am a bit sceptical on the ZK part of the story. The 3.8 release is supposed to be the last one supporting ZK so I don't really see how we could bring it to ZK, knowing that we don't plan to do a 3.9 release (current pl

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi, I agree with Chia-Ping, I think we could drop the ZK variant altogether, especially if this is not going to make it in 3.8.0. Even if we end up needing a 3.9.0 release, I wouldn't write a bunch of new ZooKeeper-related code in that release to delete it all right after in 4.0. Thanks, Mickael

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hello! I am closing this vote as ACCEPTED with 3 binding +1 (Luke, Chia-Ping and Satish) and 1 non-binding +1 (Kamal) - thank you for the reviews! Realistically, I don't think I have the bandwidth to get this in 3.8.0. Due to this, I will mark tentatively the Zookeeper part for 3.9 if the communi

Re: [VOTE] KIP-950: Tiered Storage Disablement

+1 Thanks Christo for addressing the review comments. We can update the KIP for any minor comments/clarifications. On Thu, 16 May 2024 at 15:21, Luke Chen wrote: > > Thanks Chia-Ping! > Since ZK is going to be removed, I agree the KRaft part has higher priority. > But if Christo or the community

Re: [VOTE] KIP-950: Tiered Storage Disablement

Thanks Chia-Ping! Since ZK is going to be removed, I agree the KRaft part has higher priority. But if Christo or the community contributor has spare time, it's good to have ZK part, too! Thanks. Luke On Thu, May 16, 2024 at 5:45 PM Chia-Ping Tsai wrote: > +1 but I prefer to ship it to KRaft onl

Re: [VOTE] KIP-950: Tiered Storage Disablement

+1 but I prefer to ship it to KRaft only. I do concern that community have enough time to accept more feature in 3.8 :( Best, Chia-Ping On 2024/05/14 15:20:50 Christo Lolov wrote: > Heya! > > I would like to start a vote on KIP-950: Tiered Storage Disablement in > order to catch the last Kafka

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi Christo, Thanks for the KIP and incorporating the review comments. Please update the KIP with the latest details. +1 (non-binding). Thanks, Kamal On Wed, May 15, 2024 at 3:18 PM Luke Chen wrote: > Hi Christo, > > In addition to the minor comments left in the discussion thread, it LGTM. >

Re: [VOTE] KIP-950: Tiered Storage Disablement

Hi Christo, In addition to the minor comments left in the discussion thread, it LGTM. +1 from me. Thank you. Luke On Tue, May 14, 2024 at 11:21 PM Christo Lolov wrote: > Heya! > > I would like to start a vote on KIP-950: Tiered Storage Disablement in > order to catch the last Kafka release ta

[VOTE] KIP-950: Tiered Storage Disablement

Heya! I would like to start a vote on KIP-950: Tiered Storage Disablement in order to catch the last Kafka release targeting Zookeeper - https://cwiki.apache.org/confluence/display/KAFKA/KIP-950%3A++Tiered+Storage+Disablement Best, Christo