You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "HenryCaiHaiying (via GitHub)" <gi...@apache.org> on 2026/05/20 18:39:01 UTC
[PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
HenryCaiHaiying opened a new pull request, #16453:
URL: https://github.com/apache/iceberg/pull/16453
This PR addresses the issue mentioned in https://github.com/apache/iceberg/issues/16361
Previously, each DATA_COMPLETE envelope triggered a re-scan of the entire readyBuffer to count received partitions, making per-commit work O(N^2) in the number of buffered messages. Under control-topic backlog this compounded the backlog and made recovery harder.
The fix: maintain a running receivedPartitionCount that is incremented in addReady() and reset in endCurrentCommit(). isCommitReady() becomes a constant-time comparison against expectedPartitionCount.
The change looks simple but we also verified the edge cases on when the current commit failed and a new start_commit starts, also in the situation when there is 2 Coordinator running (the previous Coordinator didn't terminate and comes back as a zombie coordinator). In both situations, we maintain the logic before and post the code change. Added 2 new unit tests to cover the situation before and after the commit and a situation when there is a DATA message generated from a zombie coordinator.
Closes #16361
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "HenryCaiHaiying (via GitHub)" <gi...@apache.org>.
HenryCaiHaiying commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4663135308
@laskoviymishka I did the rebase/merge from latest, but CI is still failing. I think the CVE-Scan workflow failure is from upstream Netty vulnerabilities which is from the recent netty library version bump.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4733614790
@HenryCaiHaiying i think it's good to merge! Thanks for that!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4662815636
@HenryCaiHaiying i'm fightiing CI now :D
can you maybe rebase to latest master>
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4674957108
i'll wait for @bryanck review before merge
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "HenryCaiHaiying (via GitHub)" <gi...@apache.org>.
HenryCaiHaiying commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4537257272
Thanks @pvary for taking a look. Who would be a good person to review this PR?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "AnatolyPopov (via GitHub)" <gi...@apache.org>.
AnatolyPopov commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4577903956
Thanks @HenryCaiHaiying for the PR!
The CommitState change looks good to me and my only concern is the comment I added above regarding `offset.flush.interval.ms` that also seems to be totally unrelated to this PR. After addressing that I think it will be merge-able.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "HenryCaiHaiying (via GitHub)" <gi...@apache.org>.
HenryCaiHaiying commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4674875893
@laskoviymishka The CI run is clean now.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "HenryCaiHaiying (via GitHub)" <gi...@apache.org>.
HenryCaiHaiying commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4662010999
Thanks. Will it get merged soon?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "AnatolyPopov (via GitHub)" <gi...@apache.org>.
AnatolyPopov commented on code in PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#discussion_r3324864327
##########
kafka-connect/kafka-connect-runtime/src/integration/java/org/apache/iceberg/connect/IntegrationTestBase.java:
##########
@@ -201,7 +201,8 @@ protected KafkaConnectUtils.Config createCommonConfig(boolean useSchema) {
.config("value.converter.schemas.enable", useSchema)
.config("iceberg.control.commit.interval-ms", 1000)
.config("iceberg.control.commit.timeout-ms", Integer.MAX_VALUE)
- .config("iceberg.kafka.auto.offset.reset", "earliest");
+ .config("iceberg.kafka.auto.offset.reset", "earliest")
+ .config("offset.flush.interval.ms", 500);
Review Comment:
Are you sure it helps with the tests stablilization? in this case it will be passed as part of the connector config but I sink it is worker-only config so will it be just ignored? Also the same value is specified in the worker config here https://github.com/apache/iceberg/blob/main/kafka-connect/kafka-connect-runtime/docker/docker-compose.yml#L108
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4532135265
@HenryCaiHaiying: That might be some collateral. I don't have deep enough knowledge about Kafka Connect to review specific changes 😢
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka merged PR #16453:
URL: https://github.com/apache/iceberg/pull/16453
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "bryanck (via GitHub)" <gi...@apache.org>.
bryanck commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4663460100
I'll take a look once the build is fixed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "HenryCaiHaiying (via GitHub)" <gi...@apache.org>.
HenryCaiHaiying commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4733644853
Thanks.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4674356281
@HenryCaiHaiying can you rebase on master one more time? CI issue seems to be fixed there
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4572348625
I'll take a deeper look
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "HenryCaiHaiying (via GitHub)" <gi...@apache.org>.
HenryCaiHaiying commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4511817336
@manuzhang @bryanck It seems you are actively reviewing PRs in Kafka-Connect path, do you mind taking a look at this simple fix PR?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "HenryCaiHaiying (via GitHub)" <gi...@apache.org>.
HenryCaiHaiying commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4693131641
@bryanck Do you have time to take a look at the PR?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "HenryCaiHaiying (via GitHub)" <gi...@apache.org>.
HenryCaiHaiying commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4733609881
@bryanck @laskoviymishka Ping ...
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "HenryCaiHaiying (via GitHub)" <gi...@apache.org>.
HenryCaiHaiying commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4606144386
@AnatolyPopov I removed offset.flush.interval.ms setting introduced earlier. I added offset.flush.interval.ms setting in the earlier commit to make the flaky integration tests stable. But I think the root cause of why those integration tests failing is fixed by this PR: https://github.com/apache/iceberg/pull/16438, so remove my custom setting.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Kafka Connect: Make CommitState.isCommitReady() O(1) [iceberg]
Posted by "HenryCaiHaiying (via GitHub)" <gi...@apache.org>.
HenryCaiHaiying commented on PR #16453:
URL: https://github.com/apache/iceberg/pull/16453#issuecomment-4521423452
@pvary It seems you are actively reviewing PRs in Kafka-Connect path, do you mind taking a look at this simple fix PR?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org