[improve] [test] add end to end deduplication test.

[thetumbled]

Motivation

Message deduplication feature has been introduced into Pulsar long time ago, but the concrete problem it solve is vague.
The info i can get from the official document is that we can enable a switch in the conf file, and then we can enjoy the message deduplication feature.
But actually, there are a lot of point that we don't clarify.

1. How do we use it? Should user set the sequence id by api exposed manually, or just let the pulsar client mange it only?
2. Which concrete situation will we meet message duplication in? Can message deduplication feature handle these situations all?

Modifications

Add end to end deduplication test to clarify what problem message deduplication can handle, and what it can't.

Verifying this change

• Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as (please describe tests).

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: thetumbled#62

[eolivelli]

Lgtm

Thank you!

[BewareMyPower]

I don't recommend adding such a test that verifies a feature does not work for certain cases.

To make users know message deduplication only works for a single partition, the best way is to just add a documentation for that.

[thetumbled]

First, I think we need to update the document.
Before i research into the implementation, i read the official document and the Limitation of idempotent producer is described as:
https://pulsar.apache.org/docs/next/txn-why/

Due to the monotonic increasing sequence ID, this solution only works on a single partition and within a single producer session (that is, for producing one message), so there is no atomicity when producing multiple messages to one or multiple partitions.

In this case, if there are some failures (for example, client / broker / bookie crashes, network failure, and more) in the process of producing and receiving messages, messages are re-processed and re-delivered, which may cause data loss or data duplication:

For the producer: if the producer retries sending messages, some messages are persisted multiple times; if the producer does not retry sending messages, some messages are persisted once and other messages are lost.

For the consumer: since the consumer does not know whether the broker has received messages or not, the consumer may not retry sending acks, which causes it to receive duplicate messages.

I can't get anything clear about what problem it can't fix.

Secondly, we need a way to elaborate various cases where message dudup can work well and not. Text description can not describe the situations precisely. We need such kind of test to prove out the ability and the shortcoming of features.
WDYT, @codelipenghui @poorbarcode @eolivelli @Technoboy- @sijie @merlimat @lhotari

[BewareMyPower]

Text description can not describe the situations precisely. We need such kind of test to prove out the ability and the shortcoming of features.

Reading the tests from source code is never the first way to know a feature. The main effect of tests is to prevent breaking changes. In short, we should

• Improve the documents if the feature is unclear to users, including the limitation
• Add the tests if what the feature guarantees is not protected by tests

[thetumbled]

No matter how, we need a way to present these test code to users to show what cases it can solve and not. For example, though i have summarized the cases in the comments of the test, developers still need to read the concrete test code to be clear about what this case really is.
Even many professional developers of pulsar are clear about the limitation of this feature only after reading these tests.

[BewareMyPower]

The most proper approach might be contributing to the https://github.com/apache/pulsar-site repo

[BewareMyPower]

Could you also shorten the method name? The test testProducerDuplicationWithReceiptLostDedupEnabledAndUserControlSequenceIdMultiPartitioned even has 90 characters.

Currently, the checkstyle plugin does not work for tests somehow (see #23079). So there is no code style check for tests.

[thetumbled]

Could you also shorten the method name? The test testProducerDuplicationWithReceiptLostDedupEnabledAndUserControlSequenceIdMultiPartitioned even has 90 characters.

Currently, the checkstyle plugin does not work for tests somehow (see #23079). So there is no code style check for tests.

Done, thanks for review.