feat(s3stream): optimize the execution sequence to avoid deep depende…

[CLFutureX]

Background: Due to the fact that the callback order of CompletableFuture follows a LIFO (Last In First Out) pattern, the current implementation cannot guarantee the execution order. Furthermore, exposing CompletableFuture to external callers of S3Stream leads to deep dependencies, which subsequently affects code readability, debugging, and problem localization.

[superhx]

How this PR solve you mentioned problems: 'execution order', 'code readability', 'debugging', and 'problem localization'?

[CLFutureX]

How this PR solve you mentioned problems: 'execution order', 'code readability', 'debugging', and 'problem localization'?

How this PR solve you mentioned problems: 'execution order', 'code readability', 'debugging', and 'problem localization'?

execution order:
In the current method(S3Stream#append), upon completion of recordBatch addition, it is expected that the whenComplete method will be executed next. However, due to the exposure of the cf (CompletableFuture) to the outside, additional callback methods are subsequently attached to this completableFuture, for instance, in kafka.log.streamaspect.ElasticLogFileRecords#append where cf.whenComplete((rst, e) -> {...}) is used. Because of the LIFO (Last In, First Out) nature of these callbacks, they do not execute as expected, impacting the deletion delay of the pendingAppends collection and delaying the statistics of the appendStreamLatency metric.

The callbacks will only execute in a FIFO (First In, First Out) order if the dependent CompletableFuture has already completed by the time new callbacks are attached.

'code readability', 'debugging', and 'problem localization'
The issues of 'code readability', 'debugging', and 'problem localization' are more pronounced when dealing with deep dependencies.
Directly exposing CompletableFuture objects to the outside can create a deep dependency chain. Here, by creating a new CompletableFuture object, we break the dependency chain or, alternatively, transform the dependencies from being object-to-object to object-to-behavior dependencies.
Alternatively, one could directly expose the CompletableFuture object returned by whenComplete to the outside, but doing so would still establish a dependency between two objects.

and I suggest choosing to create a new CompletableFuture object to expose to the outside between modules, while within the module, one can opt to rely on the object returned by whenComplete to ensure the orderliness of callback methods.

Of course, these are just my personal understandings, and I welcome any corrections. Haha.

[superhx]

In the original implementation, the execution order of the code in whenComplete remains FIFO. The current logic does not depend on the execution order of whenComplete and thenAccept appended after CompletableFuture.

Return a CompletableFuture that appends whenComplete after it can execute the whenComplete logic N microseconds earlier. It‘s an acceptable change.

However, I believe that adding extra CompleteFuture to break the chain does not bring more convenience for debugging or heap dump investigation.

[CLFutureX]

In the original implementation, the execution order of the code in whenComplete remains FIFO. The current logic does not depend on the execution order of whenComplete and thenAccept appended after CompletableFuture.

Return a CompletableFuture that appends whenComplete after it can execute the whenComplete logic N microseconds earlier. It‘s an acceptable change.

However, I believe that adding extra CompleteFuture to break the chain does not bring more convenience for debugging or heap dump investigation.

Thank you for your reply. You are right. The execution of dependencies for the CompletableFuture object returned by whenComplete is FIFO