Re: [VOTE] KIP-1031: Control offset translation in MirrorSourceConnector

2024-04-24 Thread Omnia Ibrahim
Thanks Igor, I’ll conclude the vote with 3 binding votes from Mickael Maison, 
Chris Egerton and Igor Soarez.
Thanks everyone 

> On 24 Apr 2024, at 15:11, Igor Soarez  wrote:
> 
> Hi Omnia,
> 
> Thanks for your answers, and I see you've updated the KIP so thanks for the 
> changes too.
> 
> +1 (binding), thanks for the KIP
> 
> --
> Igor



Re: [VOTE] KIP-1031: Control offset translation in MirrorSourceConnector

2024-04-24 Thread Igor Soarez
Hi Omnia,

Thanks for your answers, and I see you've updated the KIP so thanks for the 
changes too.

+1 (binding), thanks for the KIP

--
Igor


Re: [VOTE] KIP-1031: Control offset translation in MirrorSourceConnector

2024-04-24 Thread Omnia Ibrahim
I updated the KIP as well to briefly explain how offset.lag.max would help 
latency. Please let me know if the KIP now looks better? 

> On 24 Apr 2024, at 11:49, Omnia Ibrahim  wrote:
> 
> Thanks Igor for the suggestions. I updated the KIP with some of them. 
> 
>> s/recored's offset/recorded offsets
> I actually mean record’s offset and not recorded offset. 
> MirrorSourceConnector store the offset of the committed record on the 
> destination cluster in an internal topic. 
> 
>> 12. The use of the word "customers" seems a bit odd to me in this context.
>> Did you perhaps mean one of "use-cases", "users" or "operators”?
> Changed this to use-cases 
> 
>> 13. "They still left with part#1 of this feature which add cost to
>> the progress of their replication."
>> I'm unsure what this means. Do you mean to say that
>> MirrorCheckpointConnector is disabled but MirrorSourceConnector is not?
>> Could you also clarify where the additional cost comes from?
> Yes currently MM2 can disable step 2 of this feature by not running 
> MirrorCheckpointConnector or by disabling emit.checkpoints.enabled  and 
> sync.group.offsets.enabled  however as long as MirrorSourceConnector is 
> running this 1st step of the feature will never be disabled and this feature 
> is costing 1. Create internal topic 2. Latency as we need to produce/queue 
> translated offsets 
>> 14. This is probably more ignorance of mine: it doesn't seem obvious in
>> the KIP how increasing offset.lag.max  to INT_MAX helps reduce latency.
> 
> Based on my knowledge and understanding, offset.lag.max controls the size of 
> queued offsets to be synced which are stored in `pendingOffsetSyncs`. Then
> 1. MM2 will try to send these periodically in `commitRecord` using 
> "MirrorSourceTask ::firePendingOffsetSyncs”. However any as `commitRecord` 
> isn’t blocker then any failed offset sync will be skipped.
> 2. To fix this then in `commit` the connector which is a blocker method will 
> retry publish any skipped offsets.   
> 
> Now If we sit  offset.lag.max  to high value then translated queued offset 
> wouldn’t be considered stale and as a result wouldn’t be added to 
> `pendingOffsetSyncs` which what `firePendingOffsetSyncs` will try to commit. 
> Hope this clarify it.
> 
>> On 20 Apr 2024, at 09:41, Igor Soarez  wrote:
>> 
>> Hi Omnia,
>> 
>> Thanks for this KIP.
>> 
>> 11. These seem to me to be small misspellings, please double-check:
>> s/MM2 main features/MM2's main features
>> s/syncing consumer group offset/syncing consumer group offsets
>> s/relays/relies
>> s/recored's offset/recorded offsets
>> s/clusters without need for/clusters without the need for
>> s/creating internal topic./creating an internal topic.
>> s/This KIP propose that/This KIP proposes that
>> 
>> 12. The use of the word "customers" seems a bit odd to me in this context.
>> Did you perhaps mean one of "use-cases", "users" or "operators"?
>> 
>> 13. "They still left with part#1 of this feature which add cost to
>> the progress of their replication."
>> I'm unsure what this means. Do you mean to say that
>> MirrorCheckpointConnector is disabled but MirrorSourceConnector is not?
>> Could you also clarify where the additional cost comes from?
>> 
>> 14. This is probably more ignorance of mine: it doesn't seem obvious in
>> the KIP how increasing offset.lag.max  to INT_MAX helps reduce latency.
>> I'm guessing it's related to KAFKA-14610 but after having a look I
>> still couldn't understand why.
>> 
>> 
>> --
>> Igor
>> 
>> On Wed, Apr 17, 2024, at 3:22 PM, Omnia Ibrahim wrote:
>>> Thanks Chris and Mickael for the votes. 
>>> Can I please get one last +1 binding vote please?
>>> 
>>> Thanks
>>> Omnia
>>> 
 On 12 Apr 2024, at 13:21, Chris Egerton  wrote:
 
 +1 (binding), thanks Omnia!
 
 On Fri, Apr 12, 2024, 03:46 Mickael Maison  
 wrote:
 
> Hi Omnia,
> 
> +1 (binding), thanks for the KIP!
> 
> Mickael
> 
> On Fri, Apr 12, 2024 at 9:01 AM Omnia Ibrahim 
> wrote:
>> 
>> Hi everyone, I would like to start a voting thread for KIP-1031: Control
> offset translation in MirrorSourceConnector
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1031%3A+Control+offset+translation+in+MirrorSourceConnector
>> 
>> For comments or feedback please check the discussion thread here
> https://lists.apache.org/thread/ym6zr0wrhglft5c000x9c8ych098s7h6
>> 
>> Thanks
>> Omnia
>> 
> 
>>> 
>>> 
> 



Re: [VOTE] KIP-1031: Control offset translation in MirrorSourceConnector

2024-04-24 Thread Omnia Ibrahim
Thanks Igor for the suggestions. I updated the KIP with some of them. 

> s/recored's offset/recorded offsets
I actually mean record’s offset and not recorded offset. MirrorSourceConnector 
store the offset of the committed record on the destination cluster in an 
internal topic. 

> 12. The use of the word "customers" seems a bit odd to me in this context.
> Did you perhaps mean one of "use-cases", "users" or "operators”?
Changed this to use-cases 

> 13. "They still left with part#1 of this feature which add cost to
> the progress of their replication."
> I'm unsure what this means. Do you mean to say that
> MirrorCheckpointConnector is disabled but MirrorSourceConnector is not?
> Could you also clarify where the additional cost comes from?
Yes currently MM2 can disable step 2 of this feature by not running 
MirrorCheckpointConnector or by disabling emit.checkpoints.enabled  and 
sync.group.offsets.enabled  however as long as MirrorSourceConnector is running 
this 1st step of the feature will never be disabled and this feature is costing 
1. Create internal topic 2. Latency as we need to produce/queue translated 
offsets 
> 14. This is probably more ignorance of mine: it doesn't seem obvious in
> the KIP how increasing offset.lag.max  to INT_MAX helps reduce latency.

Based on my knowledge and understanding, offset.lag.max controls the size of 
queued offsets to be synced which are stored in `pendingOffsetSyncs`. Then
1. MM2 will try to send these periodically in `commitRecord` using 
"MirrorSourceTask ::firePendingOffsetSyncs”. However any as `commitRecord` 
isn’t blocker then any failed offset sync will be skipped.
2. To fix this then in `commit` the connector which is a blocker method will 
retry publish any skipped offsets.   

Now If we sit  offset.lag.max  to high value then translated queued offset 
wouldn’t be considered stale and as a result wouldn’t be added to 
`pendingOffsetSyncs` which what `firePendingOffsetSyncs` will try to commit. 
Hope this clarify it.

> On 20 Apr 2024, at 09:41, Igor Soarez  wrote:
> 
> Hi Omnia,
> 
> Thanks for this KIP.
> 
> 11. These seem to me to be small misspellings, please double-check:
> s/MM2 main features/MM2's main features
> s/syncing consumer group offset/syncing consumer group offsets
> s/relays/relies
> s/recored's offset/recorded offsets
> s/clusters without need for/clusters without the need for
> s/creating internal topic./creating an internal topic.
> s/This KIP propose that/This KIP proposes that
> 
> 12. The use of the word "customers" seems a bit odd to me in this context.
> Did you perhaps mean one of "use-cases", "users" or "operators"?
> 
> 13. "They still left with part#1 of this feature which add cost to
> the progress of their replication."
> I'm unsure what this means. Do you mean to say that
> MirrorCheckpointConnector is disabled but MirrorSourceConnector is not?
> Could you also clarify where the additional cost comes from?
> 
> 14. This is probably more ignorance of mine: it doesn't seem obvious in
> the KIP how increasing offset.lag.max  to INT_MAX helps reduce latency.
> I'm guessing it's related to KAFKA-14610 but after having a look I
> still couldn't understand why.
> 
> 
> --
> Igor
> 
> On Wed, Apr 17, 2024, at 3:22 PM, Omnia Ibrahim wrote:
>> Thanks Chris and Mickael for the votes. 
>> Can I please get one last +1 binding vote please?
>> 
>> Thanks
>> Omnia
>> 
>>> On 12 Apr 2024, at 13:21, Chris Egerton  wrote:
>>> 
>>> +1 (binding), thanks Omnia!
>>> 
>>> On Fri, Apr 12, 2024, 03:46 Mickael Maison  wrote:
>>> 
 Hi Omnia,
 
 +1 (binding), thanks for the KIP!
 
 Mickael
 
 On Fri, Apr 12, 2024 at 9:01 AM Omnia Ibrahim 
 wrote:
> 
> Hi everyone, I would like to start a voting thread for KIP-1031: Control
 offset translation in MirrorSourceConnector
 https://cwiki.apache.org/confluence/display/KAFKA/KIP-1031%3A+Control+offset+translation+in+MirrorSourceConnector
> 
> For comments or feedback please check the discussion thread here
 https://lists.apache.org/thread/ym6zr0wrhglft5c000x9c8ych098s7h6
> 
> Thanks
> Omnia
> 
 
>> 
>> 



Re: [VOTE] KIP-1031: Control offset translation in MirrorSourceConnector

2024-04-20 Thread Igor Soarez
Hi Omnia,

Thanks for this KIP.

11. These seem to me to be small misspellings, please double-check:
s/MM2 main features/MM2's main features
s/syncing consumer group offset/syncing consumer group offsets
s/relays/relies
s/recored's offset/recorded offsets
s/clusters without need for/clusters without the need for
s/creating internal topic./creating an internal topic.
s/This KIP propose that/This KIP proposes that

12. The use of the word "customers" seems a bit odd to me in this context.
Did you perhaps mean one of "use-cases", "users" or "operators"?

13. "They still left with part#1 of this feature which add cost to
the progress of their replication."
I'm unsure what this means. Do you mean to say that
MirrorCheckpointConnector is disabled but MirrorSourceConnector is not?
Could you also clarify where the additional cost comes from?

14. This is probably more ignorance of mine: it doesn't seem obvious in
the KIP how increasing offset.lag.max  to INT_MAX helps reduce latency.
I'm guessing it's related to KAFKA-14610 but after having a look I
still couldn't understand why.


--
Igor

On Wed, Apr 17, 2024, at 3:22 PM, Omnia Ibrahim wrote:
> Thanks Chris and Mickael for the votes. 
> Can I please get one last +1 binding vote please?
> 
> Thanks
> Omnia
> 
> > On 12 Apr 2024, at 13:21, Chris Egerton  wrote:
> > 
> > +1 (binding), thanks Omnia!
> > 
> > On Fri, Apr 12, 2024, 03:46 Mickael Maison  wrote:
> > 
> >> Hi Omnia,
> >> 
> >> +1 (binding), thanks for the KIP!
> >> 
> >> Mickael
> >> 
> >> On Fri, Apr 12, 2024 at 9:01 AM Omnia Ibrahim 
> >> wrote:
> >>> 
> >>> Hi everyone, I would like to start a voting thread for KIP-1031: Control
> >> offset translation in MirrorSourceConnector
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1031%3A+Control+offset+translation+in+MirrorSourceConnector
> >>> 
> >>> For comments or feedback please check the discussion thread here
> >> https://lists.apache.org/thread/ym6zr0wrhglft5c000x9c8ych098s7h6
> >>> 
> >>> Thanks
> >>> Omnia
> >>> 
> >> 
> 
> 


Re: [VOTE] KIP-1031: Control offset translation in MirrorSourceConnector

2024-04-17 Thread Omnia Ibrahim
Thanks Chris and Mickael for the votes. 
Can I please get one last +1 binding vote please?

Thanks
Omnia

> On 12 Apr 2024, at 13:21, Chris Egerton  wrote:
> 
> +1 (binding), thanks Omnia!
> 
> On Fri, Apr 12, 2024, 03:46 Mickael Maison  wrote:
> 
>> Hi Omnia,
>> 
>> +1 (binding), thanks for the KIP!
>> 
>> Mickael
>> 
>> On Fri, Apr 12, 2024 at 9:01 AM Omnia Ibrahim 
>> wrote:
>>> 
>>> Hi everyone, I would like to start a voting thread for KIP-1031: Control
>> offset translation in MirrorSourceConnector
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1031%3A+Control+offset+translation+in+MirrorSourceConnector
>>> 
>>> For comments or feedback please check the discussion thread here
>> https://lists.apache.org/thread/ym6zr0wrhglft5c000x9c8ych098s7h6
>>> 
>>> Thanks
>>> Omnia
>>> 
>> 



Re: [VOTE] KIP-1031: Control offset translation in MirrorSourceConnector

2024-04-12 Thread Chris Egerton
+1 (binding), thanks Omnia!

On Fri, Apr 12, 2024, 03:46 Mickael Maison  wrote:

> Hi Omnia,
>
> +1 (binding), thanks for the KIP!
>
> Mickael
>
> On Fri, Apr 12, 2024 at 9:01 AM Omnia Ibrahim 
> wrote:
> >
> > Hi everyone, I would like to start a voting thread for KIP-1031: Control
> offset translation in MirrorSourceConnector
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1031%3A+Control+offset+translation+in+MirrorSourceConnector
> >
> > For comments or feedback please check the discussion thread here
> https://lists.apache.org/thread/ym6zr0wrhglft5c000x9c8ych098s7h6
> >
> > Thanks
> > Omnia
> >
>


Re: [VOTE] KIP-1031: Control offset translation in MirrorSourceConnector

2024-04-12 Thread Mickael Maison
Hi Omnia,

+1 (binding), thanks for the KIP!

Mickael

On Fri, Apr 12, 2024 at 9:01 AM Omnia Ibrahim  wrote:
>
> Hi everyone, I would like to start a voting thread for KIP-1031: Control 
> offset translation in MirrorSourceConnector 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1031%3A+Control+offset+translation+in+MirrorSourceConnector
>
> For comments or feedback please check the discussion thread here 
> https://lists.apache.org/thread/ym6zr0wrhglft5c000x9c8ych098s7h6
>
> Thanks
> Omnia
>


[VOTE] KIP-1031: Control offset translation in MirrorSourceConnector

2024-04-12 Thread Omnia Ibrahim
Hi everyone, I would like to start a voting thread for KIP-1031: Control offset 
translation in MirrorSourceConnector 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-1031%3A+Control+offset+translation+in+MirrorSourceConnector

For comments or feedback please check the discussion thread here 
https://lists.apache.org/thread/ym6zr0wrhglft5c000x9c8ych098s7h6

Thanks
Omnia