Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-04-11 Thread Kamal Chandraprakash
> Is making it optional a good solution? Or should we recover the snapshot if not found before uploading it? IMO, we should recover the snapshot if it is not found. > But what if the topic is created before v2.8.0 and old log segments are deleted, how could we recover all the producer snapshot

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-04-04 Thread Luke Chen
Hi Kamal, Thanks for sharing! I didn't know about the change before v2.8. If that's the case, then we have to reconsider the solution of this PR. Is making it optional a good solution? Or should we recover the snapshot if not found before uploading it? But what if the topic is created before

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-04-03 Thread Arpit Goyal
Thanks @Kamal Chandraprakash Greg Harris I currently do not have detailed understanding on the behaviour when empty producer snapshot restored. I will try to test out the behaviour.Meanwhile I would request other community members if they can chime in and assist if they are already aware of the

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-26 Thread Kamal Chandraprakash
Hi, Sorry for the late reply. Greg has raised some good points: > Does an empty producer snapshot have the same behavior as a non-existent snapshot when restored? Producer snapshots maintain the status about the ongoing txn to either COMMIT/ABORT the transaction. In the older version (<2.8), we

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-25 Thread Greg Harris
Hi Arpit, I think creating empty producer snapshots would be backwards-compatible for the tiered storage plugins, but I'm not aware of what the other compatibility/design concerns might be. Maybe you or another reviewer can answer these questions: 1. Does an empty producer snapshot have the same

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-23 Thread Arpit Goyal
Yes Luke, I am also in favour of creating producer snapshot at run time if foundEmpty as this would only be required for topics migrated from < 2.8 version. This will not break the existing contract with the plugin. Yes, metrics do not make sense here as of now. Greg, @Kamal Chandraprakash WDYT

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-23 Thread Luke Chen
Hi Arpit, I'm in favor of creating an empty producer snapshot since it's only for topics <= v2.8. About the metric, I don't know what we expect users to know. I think we can implement with the empty producer snapshot method, without the metric. And add them if users are requested it. WDYT? Thank

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-22 Thread Arpit Goyal
Hi Team, Any further comments or suggestions on the possible approaches discussed above. On Tue, Mar 19, 2024, 09:55 Arpit Goyal wrote: > @Luke Chen @Kamal Chandraprakash @Greg > Harris Any suggestion on the above two possible approaches. > On Sun, Mar 17, 2024, 13:36 Arpit Goyal wrote: > >>

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-18 Thread Arpit Goyal
@Luke Chen @Kamal Chandraprakash @Greg Harris Any suggestion on the above two possible approaches. On Sun, Mar 17, 2024, 13:36 Arpit Goyal wrote: > >>> In summary , There are two possible solution to handle the above > scenario when producer snapshot file found to be null > > 1. *Generate

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-17 Thread Arpit Goyal
> > >> In summary , There are two possible solution to handle the above scenario when producer snapshot file found to be null 1. *Generate empty producer snapshot file at run time when copying LogSegment* - This will not require any backward compatibility dependencies with the plugin.

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-15 Thread Arpit Goyal
Hi All, I was looking at one of the plugin implementation s and Indeed it is expected to encounter a

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-15 Thread Kamal Chandraprakash
Hi Arpit, Thanks for the KIP! There is an open ticket [1] to generate the empty producer snapshot for segments which lacks one. Tiered storage is supported from IBP 2.8-IV1 which mandates that all the segments should have the producer snapshots. If we make the producer snapshot optional, then we

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-15 Thread Luke Chen
Hi Arpit, Thanks for the KIP! I agree with Greg that we should make it clear about backward Compatibility. Since you don't have permission to edit the KIP, you could think about it and write in the email thread directly. Questions: 1. Could you explain what will happen if one partition created

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-14 Thread Greg Harris
Hi Arpit, Thanks for the clarification. Replying here without updating the KIP is fine for now. I disagree with your evaluation of the backwards compatibility. If you change the return type of a method, that breaks both source and binary compatibility. After upgrading, plugin implementations

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-14 Thread Arpit Goyal
Hi Greg, I do not have access to update the KIP , Divij is helping me to do it. Meanwhile let me update your queries here. Backward compatibility: These changes will not impact the existing functionality as the existing behaviour always expects producer snapshot files to be present for a given

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-13 Thread Greg Harris
Hi Arpit, Thanks for the KIP! I am not familiar with the necessity of producer snapshots, but your explanation sounds like this should be made optional. Can you expand the KIP to include the changes that need to be made to the constructor and getter, and explain more about backwards

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-13 Thread Arpit Goyal
Hi all, I just wanted to bump up this thread. The KIP introduces a really small change and it would not take much of the time reviewing it. This change would enable kafka users to use tiered storage features seamlessly for the topics migrated from < 2.8 version which currently failed with

Re: [DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-11 Thread Arpit Goyal
Hi All, Just a Reminder, KIP-1026 is open for discussion. Thanks and Regards Arpit Goyal 8861094754 On Sat, Mar 9, 2024 at 9:27 AM Arpit Goyal wrote: > Hi All, > > I have created KIP-1026 for handling producerSnapshot empty scenarios > when the topic is upgraded from the kafka < 2.8 version.

[DISCUSS] KIP-1026: Handling producer snapshot when upgrading from < v2.8.0 for Tiered Storage

2024-03-08 Thread Arpit Goyal
Hi All, I have created KIP-1026 for handling producerSnapshot empty scenarios when the topic is upgraded from the kafka < 2.8 version. https://cwiki.apache.org/confluence/display/KAFKA/KIP-1026%3A+Handling+producer+snapshot+when+upgrading+from+%3C+v2.8.0+for+Tiered+Storage Feedback and