[improve][client] Move acknowledge APIs to another interface and improve docs

[BewareMyPower]

Motivation

Currently there are many acknowledge methods in Consumer interface, including:

• 7 overloads of acknowledgeAsync
• 4 overloads of acknowledge
• 3 overloads of acknowledgeCumulativeAsync
• 2 overloads of acknowledgeCumulative

The JavaDocs of these APIs are also massive and duplicated. For example, some terms are not for application users like "pending ack", which should not appear in API Docs.

Modifications

• Add a new interface MessageAcknowledger and move the acknowledge APIs into this new interface.
• Improve the API docs. For some overload methods, change them to default methods so that duplicated description can be avoided.

The new API docs should be much more clear than before. Though this PR adds a new API, the API compatibility is guaranteed.

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
• Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository: BewareMyPower#9

[eolivelli]

Is it API compatible with applications compiled against the previous version of the API ? is it a binary compatible change ?

There are many components (Functions, Sinks, Sources..) that inherit the Pulsar API from the container, we must ensure that changing the API jars doesn't break compatibility and requires rebuilding the applications

[BewareMyPower]

we must ensure that changing the API jars doesn't break compatibility and requires rebuilding the applications

Is there any way to verify it?

[BewareMyPower]

@eolivelli I wrote a function just now with the 2.10.2 dependency.

package org.example;

import org.apache.pulsar.functions.api.Context;
import org.apache.pulsar.functions.api.Function;

public class StringLengthExample implements Function<String, Integer> {

    @Override
    public Integer process(String input, Context context) throws Exception {
        return input.length();
    }
}

    <dependency>
      <groupId>org.apache.pulsar</groupId>
      <artifactId>pulsar-functions-api</artifactId>
      <version>2.10.2</version>
    </dependency>

Then I created the function in the branch of this PR and it works well.

./bin/pulsar-admin functions create \
 --jar ./PulsarFunctionDemo-1.0-SNAPSHOT.jar \
 --classname org.example.StringLengthExample \
 --tenant public \
 --namespace default \
 --name word-count \
 --inputs persistent://public/default/sentences \
 --output persistent://public/default/count

./bin/pulsar-client produce sentences -m "hello world java"

$ ./bin/pulsar-client consume -s sub count -n 0
...
----- got message -----
key:[null], properties:[__pfn_input_msg_id__=CAsQADAA, __pfn_input_topic__=persistent://public/default/sentences], content:16

Could it verify the API and binary compatibility?

[BewareMyPower]

I added the Consumer related API invocations, it also worked well.

        final PulsarClient client = PulsarClient.builder().serviceUrl("pulsar://localhost:6650").build();
        final Consumer<String> consumer = client.newConsumer(Schema.STRING)
                .topic("xxx")
                .subscriptionName("sub")
                .subscribe();
        consumer.acknowledge(MessageId.earliest);
        consumer.close();
        client.close();
        return input.length();

The change of this PR is similar to changing the API from

public interface A {
    void f();
    void f(int x);
    void g();
}

to

public interface F {
    void f(int x);
    // actually the default method is always overrided because the
    // previous implementation doesn't make it default
    default void f() {
        f(0);
    }
}

public interface A extends F {
    void g();
}

[BewareMyPower]

@eolivelli Is there any more concern? If you concerned about the binary compatibility, from https://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html, we can see

here is a list of some important binary compatible changes that the Java programming language supports:
...
Moving a method upward in the class hierarchy.

We can also have a simple verification like:

.
├── AImpl.java
├── v0
│   └── A.java
└── v1
    ├── A.java
    └── F.java

where AImpl implements the interface A.

$ cat v0/A.java
public interface A {
    void f();
    void f(int x);
    void g();
}
$ cat v1/A.java
public interface A extends F {
    void g();
}
$ cat v1/F.java
public interface F {
    void f(int x);
    default void f() {
        f(100);
    }
}
$ javac AImpl.java v0/A.java
$ java -classpath $PWD/v0:$PWD AImpl
f()
f(1)
g()
$ javac v1/*.java
$ java -classpath $PWD/v1:$PWD AImpl
f()
f(1)
g()

[tisonkun]

This can be a good refactor to go. To see the value of separating interfaces, we can actually mock (write test class) against the narrow interface in testing code.

[eolivelli]

Lgtm

Thanks for double checking

[tisonkun]

/pulsarbot run-failure-checks

[codecov-commenter]

Codecov Report

Merging #18519 (93c5d47) into master (cee697b) will increase coverage by 4.09%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master   #18519      +/-   ##
============================================
+ Coverage     47.06%   51.15%   +4.09%     
+ Complexity     8956     7373    -1583     
============================================
  Files           592      409     -183     
  Lines         56313    44139   -12174     
  Branches       5844     4516    -1328     
============================================
- Hits          26503    22581    -3922     
+ Misses        26946    19109    -7837     
+ Partials       2864     2449     -415     

Flag	Coverage Δ
unittests	51.15% <ø> (+4.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...er/metadata/v2/TransactionBufferSnapshotIndex.java	0.00% <0.00%> (-77.78%)	⬇️
.../metadata/v2/TransactionBufferSnapshotSegment.java	0.00% <0.00%> (-75.00%)	⬇️
...he/pulsar/broker/service/PrecisPublishLimiter.java	0.00% <0.00%> (-73.98%)	⬇️
.../metadata/v2/TransactionBufferSnapshotIndexes.java	0.00% <0.00%> (-71.43%)	⬇️
...roker/service/schema/SchemaCompatibilityCheck.java	6.25% <0.00%> (-56.25%)	⬇️
...rvice/schema/ProtobufSchemaCompatibilityCheck.java	0.00% <0.00%> (-50.00%)	⬇️
...n/java/org/apache/pulsar/PulsarVersionStarter.java	0.00% <0.00%> (-42.31%)	⬇️
...lsar/broker/loadbalance/impl/ThresholdShedder.java	3.27% <0.00%> (-27.05%)	⬇️
...oker/transaction/buffer/metadata/v2/TxnIDData.java	0.00% <0.00%> (-20.00%)	⬇️
...ker/loadbalance/impl/LeastLongTermMessageRate.java	73.33% <0.00%> (-20.00%)	⬇️
... and 292 more