vbhanuchander-lang commented on PR #17639: URL: https://github.com/apache/iceberg/pull/17639#issuecomment-5288051565
Re-adding a readiness gate may well be the right call, but three statements in the stated root cause do not match the code, and I think they matter because they are the justification for *this* particular fix. I posted the same findings on #17193 earlier today ([comment](https://github.com/apache/iceberg/issues/17193#issuecomment-5274641634)) — apologies if they crossed with your work. **1. The removed check was on the Connect group, not `cg-control`.** #14395 deleted the `ConsumerGroupState.STABLE` guard from `hasLeaderPartition`, which describes `config.connectGroupId()` — the sink's data-topic consumer group — and its purpose was coordinator **leader election** (which task owns the first partition). It never inspected the control group, so restoring it does not "ensure the `cg-control` consumer has joined". **2. `max.poll.interval.ms` does not default to 60s — it is 300000** (`ConsumerConfig`, and the 60s figure comes from a different setting: `WorkerSinkTask.iteration()` polls with `Math.max(nextCommit - now, 0)`, where `nextCommit` advances by `offset.flush.interval.ms`, whose default is `60000L` in `WorkerConfig`. That is a Connect worker setting, not a consumer one. **3. A poll gap does not expire `session.timeout.ms`.** Since KIP-62 the consumer heartbeats from a background thread, so `session.timeout.ms` (45000) covers heartbeat loss, not time between `poll()` calls; the config that evicts a member for not polling is `max.poll.interval.ms`, which a ~60s gap does not breach. Relatedly, `task.put(...)` *is* still invoked on every cycle when the topic is idle — `WorkerSinkTask.poll()` calls `deliverMessages()` unconditionally after `convertMessages(msgs)`, with no non-empty guard — so the control consumer does keep being polled. None of this says the symptom in #17193 is not real; #11818 reports the same thing. My concern is narrower: if the mechanism is not the one described, this change may be fixing it by side effect (delaying commits until the Connect group settles) rather than addressing the cause, and it would be easy for that to look fixed in one environment and not another. The thing that would settle it is which group the reporter's `UnknownMemberIdException` actually names — `cg-control-<uuid>` (created per worker at `Worker.java:53`), `<connectGroupId>-coord` (`Coordinator.java:97`), or the Connect group itself. Those are three different groups with three different fixes. Worth asking @kuldeep0508 for that line before settling on this approach. Happy to be shown wrong on any of the three points — they are all checkable against Kafka 3.9 sources. -- 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]
