Skip to content

KAFKA-6399: Remove Streams max.poll.interval override#6509

Merged
bbejeck merged 3 commits intoapache:trunkfrom
vvcephei:KAFKA-6399-reduce-streams-poll-ms
Apr 4, 2019
Merged

KAFKA-6399: Remove Streams max.poll.interval override#6509
bbejeck merged 3 commits intoapache:trunkfrom
vvcephei:KAFKA-6399-reduce-streams-poll-ms

Conversation

@vvcephei
Copy link
Copy Markdown
Contributor

Since we now call poll during restore, we can decrease the timeout
to a reasonable value, which should help Streams make progress if
threads get stuck.

Committer Checklist (excluded from commit message)

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

@vvcephei
Copy link
Copy Markdown
Contributor Author

@guozhangwang @ableegoldman ,

Can you take a quick look at this, please?

Thanks,
-John

tempConsumerDefaultOverrides.put(ConsumerConfig.MAX_POLL_INTERVAL_MS_CONFIG, Integer.toString(Integer.MAX_VALUE));
tempConsumerDefaultOverrides.put(
ConsumerConfig.MAX_POLL_INTERVAL_MS_CONFIG,
Long.toString(Duration.ofMinutes(1).toMillis())
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.

ConsumerConfigs.MAX_POLL_INTERVAL_MS_CONFIG default value is 5 minutes, so I think it is not necessary to further decrease it to 1 minute in Streams; maybe we can just remove this override to defer to Consumer's default value?

BTW we should have a tiny KIP for this default value change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@vvcephei vvcephei changed the title KAFKA-6399: Reduce Streams max.poll.interval KAFKA-6399: Remove Streams max.poll.interval override Mar 27, 2019
Copy link
Copy Markdown
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

LGTM

@guozhangwang
Copy link
Copy Markdown
Contributor

LGTM. I think the KIP has 3 votes now and also stayed for 72+ hours. Will leave to @bbejeck to merge after @vvcephei closed the voting thread.

@vvcephei
Copy link
Copy Markdown
Contributor Author

vvcephei commented Apr 4, 2019

Thanks, all. I've closed voting and marked the KIP as accepted.

@bbejeck bbejeck merged commit 9bd0d6a into apache:trunk Apr 4, 2019
@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Apr 4, 2019

Merged #6509 into trunk.

@vvcephei vvcephei deleted the KAFKA-6399-reduce-streams-poll-ms branch April 9, 2019 20:16
jarekr pushed a commit to confluentinc/kafka that referenced this pull request Apr 18, 2019
* apache/trunk:
  MINOR: Add security considerations for remote JMX in Kafka docs (apache#6544)
  MINOR: Remove redundant access specifiers from metrics interfaces (apache#6527)
  MINOR: Correct KStream documentation (apache#6552)
  KAFKA-8013; Avoid underflow when reading a Struct from a partially correct buffer (apache#6340)
  KAFKA-8058: Fix ConnectClusterStateImpl.connectors() method (apache#6384)
  MINOR: Move common consumer tests out of abstract consumer class (apache#6548)
  KAFKA-8168; Add a generated ApiMessageType class
  KAFKA-7893; Refactor ConsumerBounceTest to reuse functionality from BaseConsumerTest (apache#6238)
  MINOR: Tighten up metadata upgrade test (apache#6531)
  KAFKA-8190; Don't update keystore modification time during validation (apache#6539)
  MINOR: Fixed a few warning in core and connects (apache#6545)
  KAFKA-7904; Add AtMinIsr partition metric and TopicCommand option (KIP-427)
  MINOR: fix throttling and status in ConnectionStressWorker
  KAFKA-8090: Use automatic RPC generation in ControlledShutdown
  KAFKA-6399: Remove Streams max.poll.interval override (apache#6509)
  KAFKA-8126: Flaky Test org.apache.kafka.connect.runtime.WorkerTest.testAddRemoveTask (apache#6475)
  HOTFIX: Update unit test for KIP-443
  KAFKA-7190: KIP-443; Remove streams overrides on repartition topics (apache#6511)
  KAFKA-8183: Add retries to WorkerUtils#verifyTopics (apache#6532)
  KAFKA-8181: Removed Avro topic from TOC on kafka (apache#6529)
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
Since we now call poll during restore, we can decrease the timeout
to a reasonable value, which should help Streams make progress if
threads get stuck.

Reviewers: Guozhang Wang <wangguoz@gmail.com>,  Bill Bejeck <bbejeck@gmail.com>
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