RATIS-2421. Gracefully cancel stream after complete in GrpcLogAppender

[symious]

What changes were proposed in this pull request?

When onCompleted() is called, the client waits for the server response to finish the RPC and release the underlying resources.

However, in some cases the server response may never arrive, which leaves the RPC stream open and prevents resources from being released.

This ticket introduces a client-side cancel() after a short grace period following onCompleted(), to ensure the RPC stream is forcefully closed and resources are cleaned up.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-2421

How was this patch tested?

Test locally.

[spacemonkd]

This schedules cancelIfStillNeeded() outside the completed.compareAndSet(...) block. Repeated onCompleted() calls can queue redundant cancel tasks.

Can this be avoided?
Also I think if it’s invoked again after shutdownCloser() it may throw exception.

[symious]

The cancel and shutdown parts can be called multiple times.

[spacemonkd]

But if it is completed already aren't subsequent cancel calls queuing tasks which are no longer required?

[symious]

If onComplete() is invoked, the scheudler should be called and shutdown later, later requests should be rejected then.

[spacemonkd]

We are doing explicit cast to ClientCallStreamObserver<>, but I am thinking can there be a case where appendEntries() can return only CallStreamObserver?
That is the method signature in GrpcServerProtocolClient.

In such a case this can throw error.
Better to accept CallStreamObserver<> and then do an instanceof check?

try {
    if (stream instanceof ClientCallStreamObserver) {
      ((ClientCallStreamObserver<AppendEntriesRequestProto>) stream).cancel(reason, cause);
    } else {
      ...handle cancel when it is not clientcallstreamobserver, something like stream.onError(...)?
    }
  } catch (Exception e) {
    LOG.warn("Failed to cancel {}", name, e);
  }

[symious]

As a client, this one should always be a ClientCallStreamObserver.

[spacemonkd]

Thanks for the patch @symious.
Just a few comments, I am not too sure in some cases so definitely another set of eyes would be good to have.
But apart from these it looks good to me

[symious]

@devabhishekpal Thank you for the review. Updated and PTAL.

[spacemonkd]

Thanks for addressing the comments @symious. Looks good to me, +1