[jira] [Commented] (RATIS-2171) Safer initialization of TransactionContext#logIndexFuture

[Tsz-wo Sze (Jira)]

    [ 
https://issues.apache.org/jira/browse/RATIS-2171?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17894889#comment-17894889
 ] 

Tsz-wo Sze commented on RATIS-2171:
-----------------------------------

Indeed, we should complete the logIndexFuture inside setLogEntry(..).
{code}
diff --git 
a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/TransactionContextImpl.java
 
b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/TransactionContextImpl.java
index e582fb9e12..dbd3f59b97 100644
--- 
a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/TransactionContextImpl.java
+++ 
b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/TransactionContextImpl.java
@@ -123,7 +123,6 @@ public class TransactionContextImpl implements 
TransactionContext {
   TransactionContextImpl(RaftPeerRole serverRole, StateMachine stateMachine, 
LogEntryProto logEntry) {
     this(serverRole, null, stateMachine, logEntry.getStateMachineLogEntry());
     setLogEntry(logEntry);
-    this.logIndexFuture.complete(logEntry.getIndex());
   }
 
   @Override
@@ -179,7 +178,6 @@ public class TransactionContextImpl implements 
TransactionContext {
     Preconditions.assertNull(logEntry, "logEntry");
     Objects.requireNonNull(stateMachineLogEntry, "stateMachineLogEntry == 
null");
 
-    logIndexFuture.complete(index);
     return setLogEntry(LogProtoUtils.toLogEntryProto(stateMachineLogEntry, 
term, index));
   }
 
@@ -190,6 +188,7 @@ public class TransactionContextImpl implements 
TransactionContext {
   private LogEntryProto setLogEntry(LogEntryProto entry) {
     this.logEntry = entry;
     this.logEntryCopy = MemoizedSupplier.valueOf(() -> 
LogProtoUtils.copy(entry));
+    this.logIndexFuture.complete(entry.getIndex());
     return entry;
   }
 
{code}

> Safer initialization of TransactionContext#logIndexFuture
> ---------------------------------------------------------
>
>                 Key: RATIS-2171
>                 URL: https://issues.apache.org/jira/browse/RATIS-2171
>             Project: Ratis
>          Issue Type: Improvement
>          Components: server
>    Affects Versions: 3.0.1
>            Reporter: JiangHua Zhu
>            Assignee: JiangHua Zhu
>            Priority: Major
>
> In TransactionContextImpl(), logIndexFuture is initialized after 
> setLogEntry(), which is very reasonable. However, in initLogEntry(), the 
> assignment of logIndexFuture is arranged before setLogEntry(), which seems to 
> be a flaw.
> TransactionContextImpl()：
> {code:java}
>   TransactionContextImpl(RaftPeerRole serverRole, StateMachine stateMachine, 
> LogEntryProto logEntry) {
>     this(serverRole, null, stateMachine, logEntry.getStateMachineLogEntry());
>     setLogEntry(logEntry);
>     this.logIndexFuture.complete(logEntry.getIndex());
>   }
> {code}
> initLogEntry()：
> {code:java}
>    ......
>     logIndexFuture.complete(index);
>     return setLogEntry(LogProtoUtils.toLogEntryProto(stateMachineLogEntry, 
> term, index));
>     ......
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)