Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-10199: Remove changelog unregister from state updater #12573

Merged
merged 1 commit into from
Sep 1, 2022

Conversation

cadonna
Copy link
Contributor

@cadonna cadonna commented Aug 31, 2022

Changelogs are already unregistered when tasks are closed.
There is no need to also unregister them in the state
updater.

In future, when we will only have the state updater without
the old code path, we should consider registering and
unregistering the changelogs within the state updater.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Changelogs are already unregistered when tasks are closed.
There is no need to also unregister them in the state
updater.

In future, when we will only have the state updater without
the old code path, we should consider registering and
unregistering the changelogs within the state updater.
@cadonna
Copy link
Contributor Author

cadonna commented Aug 31, 2022

Call for review: @wcarlson5

Copy link
Contributor

@guozhangwang guozhangwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@cadonna
Copy link
Contributor Author

cadonna commented Sep 1, 2022

Build failures are not related:

Build / JDK 11 and Scala 2.13 / org.apache.kafka.trogdor.coordinator.CoordinatorTest.testTaskRequestWithOldStartMsGetsUpdated()
Build / JDK 17 and Scala 2.13 / kafka.api.SaslClientsWithInvalidCredentialsTest.testManualAssignmentConsumerWithAutoCommitDisabledWithAuthenticationFailure()
Build / JDK 17 and Scala 2.13 / kafka.api.TransactionsTest.testCommitTransactionTimeout(String).quorum=kraft
Build / JDK 17 and Scala 2.13 / org.apache.kafka.streams.integration.EOSUncleanShutdownIntegrationTest.shouldWorkWithUncleanShutdownWipeOutStateStore[exactly_once_v2]
Build / JDK 17 and Scala 2.13 / org.apache.kafka.streams.integration.EosIntegrationTest.shouldBeAbleToRunWithTwoSubtopologies[at_least_once]
Build / JDK 17 and Scala 2.13 / org.apache.kafka.streams.integration.EosIntegrationTest.shouldWriteLatestOffsetsToCheckpointOnShutdown[at_least_once]
Build / JDK 17 and Scala 2.13 / org.apache.kafka.streams.integration.KStreamRepartitionIntegrationTest.shouldGoThroughRebalancingCorrectly[Optimization = all]

@cadonna cadonna merged commit 2bef80e into apache:trunk Sep 1, 2022
@cadonna cadonna deleted the remove_unregister_from_state_updater branch September 7, 2022 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants