kumarpritam863 commented on code in PR #17450:
URL: https://github.com/apache/iceberg/pull/17450#discussion_r3712244250
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/channel/Channel.java:
##########
@@ -53,20 +54,29 @@ abstract class Channel {
private final Admin admin;
private final Map<Integer, Long> controlTopicOffsets = Maps.newHashMap();
private final String producerId;
+ private final String channelId;
Channel(
String name,
String consumerGroupId,
IcebergSinkConfig config,
KafkaClientFactory clientFactory,
SinkTaskContext context) {
+ this.channelId = config.connectorName() + "-" + config.taskId() + "-" +
name;
this.controlTopic = config.controlTopic();
this.connectGroupId = config.connectGroupId();
this.context = context;
- String transactionalId = config.transactionalPrefix() + name +
config.transactionalSuffix();
+ String transactionalId =
+ "worker".equalsIgnoreCase(name)
+ ? config.transactionalPrefix() + name +
config.transactionalSuffix()
+ : connectGroupId + "-" + config.connectorName() + "-coord";
Review Comment:
`a new one carry different transactional ids across a rolling upgrade.` ->
This will be the case only during the upgrade and after that it will be stable.
`It also drops transactionalSuffix() (the worker path keeps it). Operators
set that suffix to isolate producers across multiple Connect clusters, so every
cluster's coordinator now collapses onto the same id and they fence each
other.` -> This still holds as connectGroupId is same as the consumerGroupId
and per kafka cluster we can have only one connect cluster with a consumer
group id and if two connectors share the consumer group id then they are
effectively in the same group.
`I'd keep the suffix on both paths and gate the new format behind config so
upgrades have a migration path. (Minor while we're here: connectGroupId already
defaults to connect-<connectorName>, so this renders as
connect-myconn-myconn-coord — the name lands twice.) wdyt?` -> yeah this is
true when we do not give consumer group id as part of connector config, but I
will change this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
