Skip to content

MINOR: KafkaBroker.brokerState should be volatile instead of AtomicReference#10080

Merged
ijuma merged 1 commit intoapache:trunkfrom
ijuma:broker-state-volatile
Feb 10, 2021
Merged

MINOR: KafkaBroker.brokerState should be volatile instead of AtomicReference#10080
ijuma merged 1 commit intoapache:trunkfrom
ijuma:broker-state-volatile

Conversation

@ijuma
Copy link
Copy Markdown
Member

@ijuma ijuma commented Feb 8, 2021

We don't need or use the additional functionality provided by
AtomicReference.

Committer Checklist (excluded from commit message)

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

…ference

We don't need or use the additional functionality provided by
AtomicReference.
@ijuma ijuma requested a review from mumrah February 8, 2021 01:38
@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Feb 8, 2021

cc @rondagostino

Comment thread core/src/main/scala/kafka/server/KafkaBroker.scala
Copy link
Copy Markdown
Member

@mumrah mumrah left a comment

Choose a reason for hiding this comment

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

LGTM

}

newGauge("BrokerState", () => brokerState.get.value())
newGauge("BrokerState", () => brokerState.value)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(this is not related to this PR) it is a bit weird to me that we expose "byte" as "broker state".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Right. To change this, it would require a KIP.

@ijuma ijuma merged commit ad541b9 into apache:trunk Feb 10, 2021
@ijuma ijuma deleted the broker-state-volatile branch February 10, 2021 15:12
ijuma added a commit to ijuma/kafka that referenced this pull request Feb 14, 2021
…e-allocations-lz4

* apache-github/trunk: (118 commits)
  KAFKA-12327: Remove MethodHandle usage in CompressionType (apache#10123)
KAFKA-12297: Make MockProducer return RecordMetadata with values as
per contract
  MINOR: Update zstd and use classes with no finalizers (apache#10120)
KAFKA-12326: Corrected regresion in MirrorMaker 2 executable
introduced with KAFKA-10021 (apache#10122)
KAFKA-12321 the comparison function for uuid type should be 'equals'
rather than '==' (apache#10098)
  MINOR: Add FetchSnapshot API doc in KafkaRaftClient (apache#10097)
  MINOR: KIP-631 KafkaConfig fixes and improvements (apache#10114)
  KAFKA-12272: Fix commit-interval metrics (apache#10102)
  MINOR: Improve confusing admin client shutdown logging (apache#10107)
  MINOR: Add BrokerMetadataListener (apache#10111)
  MINOR: Support Raft-based metadata quorums in system tests (apache#10093)
MINOR: add the MetaLogListener, LocalLogManager, and Controller
interface. (apache#10106)
  MINOR: Introduce the KIP-500 Broker lifecycle manager (apache#10095)
MINOR: Remove always-passing validation in
TestRecordTest#testProducerRecord (apache#9930)
KAFKA-5235: GetOffsetShell: Support for multiple topics and consumer
configuration override (KIP-635) (apache#9430)
MINOR: Prevent creating partition.metadata until ID can be written
(apache#10041)
  MINOR: Add RaftReplicaManager (apache#10069)
MINOR: Add ClientQuotaMetadataManager for processing QuotaRecord
(apache#10101)
  MINOR: Rename DecommissionBrokers to UnregisterBrokers (apache#10084)
MINOR: KafkaBroker.brokerState should be volatile instead of
AtomicReference (apache#10080)
  ...

clients/src/main/java/org/apache/kafka/common/record/CompressionType.java
core/src/test/scala/unit/kafka/coordinator/group/GroupMetadataManagerTest.scala
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.

3 participants