From a483bd4bf015b5b368215e0d622ff43ed317b0c7 Mon Sep 17 00:00:00 2001
From: William Song <48054931+SzyWilliam@users.noreply.github.com>
Date: Wed, 6 Sep 2023 02:19:00 +0800
Subject: [PATCH] RATIS-1884. Fix retry cache warning condition (#915)

(cherry picked from commit 4e5443a4f2e76af060c3725663de3ae73603c50d)
---
 ratis-docs/src/site/markdown/configurations.md                  | 2 ++
 .../main/java/org/apache/ratis/server/RaftServerConfigKeys.java | 1 +
 .../main/java/org/apache/ratis/server/impl/RaftServerImpl.java  | 2 +-
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/ratis-docs/src/site/markdown/configurations.md b/ratis-docs/src/site/markdown/configurations.md
index 4f6d215555..4e37cfd766 100644
--- a/ratis-docs/src/site/markdown/configurations.md
+++ b/ratis-docs/src/site/markdown/configurations.md
@@ -538,6 +538,8 @@ Note that `slowness.timeout` is use in two places:
 | **Description** | expire time of retry cache entry     |
 | **Type**        | TimeDuration                         |
 | **Default**     | 60s                                  |
+Note that we should set an expiration time longer than the total retry waiting duration of clients 
+in order to ensure exactly-once semantic.
 
 | **Property**    | `raft.server.retrycache.statistics.expire-time` |
 |:----------------|:------------------------------------------------|
diff --git a/ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java b/ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
index e561a3cd92..a8a7892dcf 100644
--- a/ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
+++ b/ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
@@ -793,6 +793,7 @@ static void setSlownessTimeout(RaftProperties properties, TimeDuration expiryTim
   interface RetryCache {
     String PREFIX = RaftServerConfigKeys.PREFIX + ".retrycache";
 
+    /** We should set expiry time longer than total client retry to guarantee exactly-once semantic */
     String EXPIRY_TIME_KEY = PREFIX + ".expirytime";
     TimeDuration EXPIRY_TIME_DEFAULT = TimeDuration.valueOf(60, TimeUnit.SECONDS);
     static TimeDuration expiryTime(RaftProperties properties) {
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
index c11a1a4c42..40a17c4e95 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
@@ -1777,7 +1777,7 @@ private CompletableFuture<Message> replyPendingRequest(
     // update the retry cache
     final CacheEntry cacheEntry = retryCache.getOrCreateEntry(invocationId);
     Preconditions.assertTrue(cacheEntry != null);
-    if (getInfo().isLeader() && !cacheEntry.isCompletedNormally()) {
+    if (getInfo().isLeader() && cacheEntry.isCompletedNormally()) {
       LOG.warn("{} retry cache entry of leader should be pending: {}", this, cacheEntry);
     }
     if (cacheEntry.isFailed()) {