Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix lazy partitioned producer might send duplicated messages #342

Merged

Conversation

BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Nov 9, 2023

Fixes #341

Motivation

When a lazy partitioned producer sends two messages, the flow is:

  1. start is called to grab the connection via grab().
  2. Generate 0 as the sequence id of the 1st message.
  3. Add the 1st message into the queue.
  4. The connection is established, msgSequenceGenerator_ is reset from 1 to 0.
  5. When sending the 2nd message, 0 is also generated as the sequence id.

Then two messages have the same sequence id.

Modifications

For lazy partitioned producers, if the internal producer is not started, sending the message in the callback of its future.

Add ChunkDedupTest#testLazyPartitionedProducer to verify it since only the tests/chunkdedup/docker-compose.yml enables the deduplication.

Fixes apache#341

### Motivation

When a lazy partitioned producer sends two messages, the flow is:
1. `start` is called to grab the connection via `grab()`.
2. Generate 0 as the sequence id of the 1st message.
3. Add the 1st message into the queuea.
4. The connection is established, `msgSequenceGenerator_` is reset from
   1 to 0.
5. When sending the 2nd message, 0 is also generated as the sequence id.

Then two messages have the same sequence id.

### Modifications

For lazy partitioned producers, if the internal producer is not started,
sending the message in the callback of its future.

Add `ChunkDedupTest#testLazyPartitionedProducer` to verify it since only
the `tests/chunkdedup/docker-compose.yml` enables the deduplication.
@BewareMyPower BewareMyPower added the bug Something isn't working label Nov 9, 2023
@BewareMyPower BewareMyPower added this to the 3.5.0 milestone Nov 9, 2023
@BewareMyPower BewareMyPower self-assigned this Nov 9, 2023
@BewareMyPower BewareMyPower merged commit bb16f24 into apache:main Nov 13, 2023
@BewareMyPower BewareMyPower deleted the bewaremypower/fix-lazy-producer-start branch November 13, 2023 02:02
@BewareMyPower BewareMyPower modified the milestones: 3.5.0, 3.4.1 Nov 16, 2023
BewareMyPower added a commit that referenced this pull request Nov 17, 2023
Fixes #341

### Motivation

When a lazy partitioned producer sends two messages, the flow is:
1. `start` is called to grab the connection via `grab()`.
2. Generate 0 as the sequence id of the 1st message.
3. Add the 1st message into the queuea.
4. The connection is established, `msgSequenceGenerator_` is reset from
   1 to 0.
5. When sending the 2nd message, 0 is also generated as the sequence id.

Then two messages have the same sequence id.

### Modifications

For lazy partitioned producers, if the internal producer is not started,
sending the message in the callback of its future.

Add `ChunkDedupTest#testLazyPartitionedProducer` to verify it since only
the `tests/chunkdedup/docker-compose.yml` enables the deduplication.

(cherry picked from commit bb16f24)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Flaky BasicEndToEndTest.testPartitionedLazyProducerConsumer
2 participants