Skip to content

MINOR: Use right enum value for broker registration change#12236

Merged
jsancio merged 1 commit intoapache:trunkfrom
dengziming:minor-BrokerRegistrationChangeRecord
Aug 2, 2022
Merged

MINOR: Use right enum value for broker registration change#12236
jsancio merged 1 commit intoapache:trunkfrom
dengziming:minor-BrokerRegistrationChangeRecord

Conversation

@dengziming
Copy link
Member

More detailed description of your change
In ClusterControlManager.replay and ReplicationControlManager, we use BrokerRegistrationFencingChange.FENCE when unfencing a broker and use BrokerRegistrationFencingChange.UNFENCE when fencing a broker, this is confusing.

Summary of testing strategy (including rationale)
Didn't change any logic.

Committer Checklist (excluded from commit message)

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

@dengziming
Copy link
Member Author

ping @dajac @cmccabe @jsancio

@dengziming dengziming force-pushed the minor-BrokerRegistrationChangeRecord branch from 06bff4e to c4f7665 Compare June 2, 2022 09:57
@jsancio
Copy link
Member

jsancio commented Jun 2, 2022

@dengziming Thanks for improving this! There are a lot of test failures. Can you take a look at those?

@jsancio
Copy link
Member

jsancio commented Jun 3, 2022

@dengziming I merge trunk to try to kick off a build.

@dengziming
Copy link
Member Author

@dengziming I merge trunk to try to kick off a build.

Thank you.

@jsancio
Copy link
Member

jsancio commented Jun 6, 2022

We need to make sure that we handle these changes depending on which commit gets merged first:

db82247#diff-7e2c147bf9c397298568e943069881bd7f01dd9c391f2a8ead700e232987e0aaL96-R107

@jsancio
Copy link
Member

jsancio commented Jun 7, 2022

@dengziming I merged KAFKA-13916. When you have sometime can you take a look at this change again?

@dengziming dengziming force-pushed the minor-BrokerRegistrationChangeRecord branch from 0a6669e to c0690fc Compare June 8, 2022 02:35
@dengziming
Copy link
Member Author

ping @jsancio

@dengziming dengziming force-pushed the minor-BrokerRegistrationChangeRecord branch from c0690fc to 76ff069 Compare June 16, 2022 08:25
@dengziming
Copy link
Member Author

cc @jsancio.

@showuon showuon mentioned this pull request Jun 23, 2022
3 tasks
@dengziming
Copy link
Member Author

cc @jsancio

Copy link
Member

@jsancio jsancio left a comment

Choose a reason for hiding this comment

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

LGTM. After searching the code, it looks like this PR updated all of the call sites. Thanks for the improvements and tests.

@jsancio jsancio merged commit f26842c into apache:trunk Aug 2, 2022
cmccabe pushed a commit that referenced this pull request Aug 3, 2022
The code used BrokerRegistrationFencingChange.FENCE when unfencing a broker and used BrokerRegistrationFencingChange.UNFENCE when fencing a broker, this is confusing. This commit flips the values of the two enums and changes their usage at all of the call sites.

Reviewers: José Armando García Sancio <jsancio@users.noreply.github.com>
ijuma added a commit to confluentinc/kafka that referenced this pull request Aug 5, 2022
…(5 August 2022)

Version related conflicts:
* Jenkinsfile
* gradle.properties
* streams/quickstart/java/pom.xml
* streams/quickstart/java/src/main/resources/archetype-resources/pom.xml
* streams/quickstart/pom.xml
* tests/kafkatest/__init__.py
* tests/kafkatest/version.py

* commit 'add7cd85baa61cd0e1430': (66 commits)
KAFKA-14136 Generate ConfigRecord for brokers even if the value is
unchanged (apache#12483)
  HOTFIX / KAFKA-14130: Reduce RackAwarenesssTest to unit Test (apache#12476)
  MINOR: Remove ARM/PowerPC builds from Jenkinsfile (apache#12380)
  KAFKA-14111 Fix sensitive dynamic broker configs in KRaft (apache#12455)
  KAFKA-13877: Fix flakiness in RackAwarenessIntegrationTest (apache#12468)
KAFKA-14129: KRaft must check manual assignments for createTopics are
contiguous (apache#12467)
KAFKA-13546: Do not fail connector validation if default topic
creation group is explicitly specified (apache#11615)
KAFKA-14122: Fix flaky test
DynamicBrokerReconfigurationTest#testKeyStoreAlter (apache#12452)
  MINOR; Use right enum value for broker registration change (apache#12236)
  MINOR; Synchronize access to snapshots' TreeMap (apache#12464)
  MINOR; Bump trunk to 3.4.0-SNAPSHOT (apache#12463)
  MINOR: Stop logging 404s at ERROR level in Connect
KAFKA-14095: Improve handling of sync offset failures in MirrorMaker
(apache#12432)
  Minor: enable index for emit final sliding window (apache#12461)
  MINOR: convert some more junit tests to support KRaft (apache#12456)
  KAFKA-14108: Ensure both JUnit 4 and JUnit 5 tests run (apache#12441)
  MINOR: Remove code of removed metric (apache#12453)
MINOR: Update comment on verifyTaskGenerationAndOwnership method in
DistributedHerder
KAFKA-14012: Add warning to closeQuietly documentation about method
references of null objects (apache#12321)
  MINOR: Fix static mock usage in ThreadMetricsTest (apache#12454)
  ...
@dengziming dengziming deleted the minor-BrokerRegistrationChangeRecord branch October 8, 2022 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants