Skip to content

Adjust broker-side observer interface so that the interface only take Java objects as argument.#8

Merged
Lincong merged 0 commit into
linkedin:2.0-lifrom
Lincong:broker-side-observer-interface-v2
Mar 30, 2019
Merged

Adjust broker-side observer interface so that the interface only take Java objects as argument.#8
Lincong merged 0 commit into
linkedin:2.0-lifrom
Lincong:broker-side-observer-interface-v2

Conversation

@Lincong

@Lincong Lincong commented Mar 29, 2019

Copy link
Copy Markdown

The previous observer interface has a method called "observe" which takes RequestChannel.Request and AbstractResponse as arguments. However, RequestChannel.Request is a class defined in Scala. Since the implementation of the observer interface is actually in Java. That means the Scala object needs to be converted in Java in the implementation. This conversion is prone to bugs, hard to read and can easily break compatibility if in the future upstream modify the structure of RequestChannel.Request, which they did before and considered MINOR: https://github.com/apache/kafka/pull/3673/files#diff-d0332a0ff31df50afce3809d90505b25R49.

Therefore, the adjusted observer interface takes 3 objects as shown below:

  • The metadata of the request including principal, header, client address. This is captured by the RequestContext object.
  • The payload of the request. This is captured by AbstractReqeust object.
  • The payload of the response. This is captured by AbstractResponse object.

@Lincong Lincong merged this pull request into linkedin:2.0-li Mar 30, 2019
earlcoder added a commit that referenced this pull request Apr 28, 2026
…#7)

The Partition.isOneAboveMinIsr method is still defined in 3.6-li but the
gauge that exposes it as a JMX/yammer metric was lost in the PR #538
squash. Restoring matches 3.0-li ReplicaManager.scala:296. Audit also
flagged the dead-code method as P1 #8 — restoring the gauge wires it
back as the original caller.
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