Skip to content

KAFKA-8284: enable static membership on KStream#6673

Merged
guozhangwang merged 4 commits into
apache:trunkfrom
abbccdda:enable_stream
May 8, 2019
Merged

KAFKA-8284: enable static membership on KStream#6673
guozhangwang merged 4 commits into
apache:trunkfrom
abbccdda:enable_stream

Conversation

@abbccdda

@abbccdda abbccdda commented May 3, 2019

Copy link
Copy Markdown

Part of KIP-345 effort. The strategy is to extract user passed in group.instance.id config and pass it in with given thread-id (because consumer is currently per-thread level).

Committer Checklist (excluded from commit message)

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

@bbejeck bbejeck added the streams label May 3, 2019
@bbejeck

bbejeck commented May 3, 2019

Copy link
Copy Markdown
Member

@abbccdda failures seem related failed for task ':clients:compileTestJava'

@abbccdda

abbccdda commented May 4, 2019

Copy link
Copy Markdown
Author

@bbejeck Thanks for the info!

@abbccdda abbccdda force-pushed the enable_stream branch 2 times, most recently from 31ac81d to 91c3ac8 Compare May 5, 2019 00:40

@guozhangwang guozhangwang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have some minor questions on whether it is necessary to define this on the streams config level, otherwise lgtm.

consumerConfigs.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "none");
}

final String streamGroupInstanceId = config.getString(ConsumerConfig.GROUP_INSTANCE_ID_CONFIG);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's move this logic into getMainConsumerConfigs directly for cleanness?

"",
Importance.MEDIUM,
CLIENT_ID_DOC)
.define(ConsumerConfig.GROUP_INSTANCE_ID_CONFIG,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have to define it on the StreamsConfig level?

Type.STRING,
null,
Importance.MEDIUM,
ConsumerConfig.GROUP_INSTANCE_ID_CONFIG)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It should be xx_DOC string.

@abbccdda

abbccdda commented May 7, 2019

Copy link
Copy Markdown
Author

Thanks for the review @guozhangwang. Updated the injection path to extract the config value from original consumer config, and refactor the getMainConsumerConfig interface a bit.

@guozhangwang guozhangwang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just one minor, otherwise lgtm.

We can merge it afterwards and green builds.

Comment thread streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java Outdated
@guozhangwang guozhangwang merged commit b0e82a6 into apache:trunk May 8, 2019
ijuma added a commit to ijuma/kafka that referenced this pull request May 8, 2019
…s-hashcode

* apache-github/trunk:
  KAFKA-8158: Add EntityType for Kafka RPC fields (apache#6503)
  MINOR: correctly parse version OffsetCommitResponse version < 3
  KAFKA-8284: enable static membership on KStream (apache#6673)
  KAFKA-8304: Fix registration of Connect REST extensions (apache#6651)
  KAFKA-8275; Take throttling into account when choosing least loaded node (apache#6619)
  KAFKA-3522: Interactive Queries must return timestamped stores (apache#6661)
  MINOR: MetricsIntegrationTest should set StreamsConfig.STATE_DIR_CONFIG (apache#6687)
  MINOR: Remove unused field in `ListenerConnectionQuota`
  KAFKA-8131; Move --version implementation into CommandLineUtils (apache#6481)
  KAFKA-8056; Use automatic RPC generation for FindCoordinator (apache#6408)
  MINOR: Remove workarounds for lz4-java bug affecting byte buffers (apache#6679)
  KAFKA-7455: Support JmxTool to connect to a secured RMI port. (apache#5968)
  MINOR: Document improvement (apache#6682)
  MINOR: Fix ThrottledReplicaListValidator doc error. (apache#6537)
  KAFKA-8306; Initialize log end offset accurately when start offset is non-zero (apache#6652)
bbejeck pushed a commit that referenced this pull request May 22, 2019
…am (#6779)

As title states. We plan to merge this to both trunk and 2.3 if it could fix the stream system tests globally.
Reference implementation: #6673

Reviewers: Guozhang Wang <wangguoz@gmail.com>, Matthias J. Sax <mjsax@apache.org>
guozhangwang pushed a commit that referenced this pull request May 22, 2019
…am (#6779)

As title states. We plan to merge this to both trunk and 2.3 if it could fix the stream system tests globally.
Reference implementation: #6673

Reviewers: Guozhang Wang <wangguoz@gmail.com>, Matthias J. Sax <mjsax@apache.org>
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
Part of KIP-345 effort. The strategy is to extract user passed in group.instance.id config and pass it in with given thread-id (because consumer is currently per-thread level).

Reviewers: Guozhang Wang <wangguoz@gmail.com>
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
…am (apache#6779)

As title states. We plan to merge this to both trunk and 2.3 if it could fix the stream system tests globally.
Reference implementation: apache#6673

Reviewers: Guozhang Wang <wangguoz@gmail.com>, Matthias J. Sax <mjsax@apache.org>
@mjsax mjsax added the kip Requires or implements a KIP label Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kip Requires or implements a KIP streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants