bump for another potential more discussion

On 2020/08/27 23:31:38, Ning Zhang <ning2008w...@gmail.com> wrote: 
> Hello Mickael,
> 
> > 1. How does offset translation work with this new sink connector?
> > Should we also include a CheckpointSinkConnector?
> 
> CheckpointSourceConnector will be re-used as the same as current. When EOS is 
> enabled, we will run 3 connectors:
> 
> MirrorSinkConnector (based on SinkConnector)
> MirrorCheckpointConnector (based on SourceConnector)
> MirrorHeartbeatConnector (based on SourceConnector)
> 
> For the last two connectors (checkpoint, heartbeat), if we do not strictly 
> require EOS, it is probably OK to use current implementation on 
> SourceConnector.
> 
> I will update the KIP to clarify this, if it sounds acceptable.
> 
> > 2. Migrating to this new connector could be tricky as effectively the
> > Connect runtime needs to point to the other cluster, so its state
> > (stored in the __connect topics) is lost. Unfortunately there is no
> > easy way today to prime Connect with offsets. Not necessarily a
> > blocking issue but this should be described as I think the current
> > Migration section looks really optimistic at the moment
> 
> totally agree. I will update the migration part with notes about potential 
> service interruption, without careful planning.
> 
> > 3. We can probably find a better name than "transaction.producer".
> > Maybe we can follow a similar pattern than Streams (which uses
> > "processing.guarantee")?
> 
> "processing.guarantee" sounds better
> 
> > 4. Transactional Ids used by the producer are generated based on the
> > task assignments. If there's a single task, if it crashes and restarts
> > it would still get the same id. Can this be an issue?
> 
> From https://tgrez.github.io/posts/2019-04-13-kafka-transactions.html, the 
> author suggests to postfix transaction.id with <topic, partition>:
> 
> "To avoid handling an external store we will use a static encoding similarly 
> as in spring-kafka:
> The transactional.id is now the transactionIdPrefix appended with 
> <group.id>.<topic>.<partition>."
> 
> I think as long as there is no more than one producer use same 
> "transaction.id" at the same time, it is OK. 
> 
> Also from my tests, this "transaction.id" assignment works fine with 
> failures. To tighten it up, I also tested to use  "connector task id" in 
> "transaction.id". The "connector task id" is typically composed of 
> connector_name and task_id, which is also unique across all connectors in a 
> KC cluster.
> 
>  > 5. The logic in the KIP creates a new transaction every time put() is
> > called. Is there a performance impact?
> 
> It could be a performance hit if the transaction batch is too small under 
> high ingestion rate. The batch size depends on how many messages that 
> consumer poll each time. Maybe we could increase "max.poll.records" to have 
> larger batch size.
> 
> Overall, thanks so much for the valuable feedback. If the responses sounds 
> good, I will do a cleanup of KIP.
> 
> On 2020/08/27 09:59:57, Mickael Maison <mickael.mai...@gmail.com> wrote: 
> > Thanks Ning for the KIP. Having stronger guarantees when mirroring
> > data would be a nice improvement!
> > 
> > A few comments:
> > 1. How does offset translation work with this new sink connector?
> > Should we also include a CheckpointSinkConnector?
> > 
> > 2. Migrating to this new connector could be tricky as effectively the
> > Connect runtime needs to point to the other cluster, so its state
> > (stored in the __connect topics) is lost. Unfortunately there is no
> > easy way today to prime Connect with offsets. Not necessarily a
> > blocking issue but this should be described as I think the current
> > Migration section looks really optimistic at the moment
> > 
> > 3. We can probably find a better name than "transaction.producer".
> > Maybe we can follow a similar pattern than Streams (which uses
> > "processing.guarantee")?
> > 
> > 4. Transactional Ids used by the producer are generated based on the
> > task assignments. If there's a single task, if it crashes and restarts
> > it would still get the same id. Can this be an issue?
> > 
> > 5. The logic in the KIP creates a new transaction every time put() is
> > called. Is there a performance impact?
> > 
> > On Fri, Aug 21, 2020 at 4:58 PM Ryanne Dolan <ryannedo...@gmail.com> wrote:
> > >
> > > Awesome, this will be a huge advancement. I also want to point out that
> > > this KIP implements MirrorSinkConnector as well, finally, which is a very
> > > often requested missing feature in my experience.
> > >
> > > Ryanne
> > >
> > > On Fri, Aug 21, 2020, 9:45 AM Ning Zhang <ning2008w...@gmail.com> wrote:
> > >
> > > > Hello, I wrote a KIP about MirrorMaker2 Exactly-once Semantics (EOS)
> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-656%3A+MirrorMaker2+Exactly-once+Semantics
> > > > At the high-level, it resembles the idea of how HDFS Sink Connector
> > > > achieves EOS across clusters by managing and storing the consumer 
> > > > offsets
> > > > in an external persistent storage, but also leverages the current Kafka 
> > > > EOS
> > > > guarantee within a single cluster. I have done some experiments 
> > > > especially
> > > > for the failure cases and I am very appreciated for comments and 
> > > > feedback
> > > > on this KIP from bigger audience.
> > > >
> > 
> 

Reply via email to