Skip to content

Update maxInFlightRequests to 5#980

Closed
mrnonz wants to merge 1 commit into
tulios:masterfrom
mrnonz:patch-1
Closed

Update maxInFlightRequests to 5#980
mrnonz wants to merge 1 commit into
tulios:masterfrom
mrnonz:patch-1

Conversation

@mrnonz

@mrnonz mrnonz commented Dec 2, 2020

Copy link
Copy Markdown

The kafkajs require Kafka version >= 0.11, so We can use maxInFlightRequests with value 5.

Here more document for guarantee with this value. https://issues.apache.org/jira/browse/KAFKA-5494

So, It's not required to set the maxInFlightRequests to 1. (It's only use for ordering message but not relate to transaction purpose).

The kafkajs require Kafka version >= 0.11, so We can use maxInFlightRequests with value 5.

Here more document for guarantee with this value. https://issues.apache.org/jira/browse/KAFKA-5494

So, It's not required to set the maxInFlightRequests to 1. (It's only use for ordering message but not relate to transaction purpose).
@Nevon

Nevon commented Dec 5, 2020

Copy link
Copy Markdown
Collaborator

While I haven't read through the entire implementation of this, I'm afraid it's not as simple as just setting it to 5. It requires that the client implement a certain behavior to allow for it first. Specifically from KAFKA-5494:

Sequence number management on the client when there are request failures. When a batch fails, future inflight batches will also fail with OutOfOrderSequenceException. This must be handled on the client with intelligent sequence reassignment. We must also deal with the fatal failure of some batch: the future batches must get different sequence numbers when the come back.

This document describes the design

Granted, I haven't looked at our transaction code for literal years, but I don't think KafkaJS is currently designed to adhere to those requirements.

@Nevon Nevon closed this Dec 5, 2020
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