szetszwo commented on code in PR #1042:
URL: https://github.com/apache/ratis/pull/1042#discussion_r1498041091
##########
ratis-server-api/src/main/java/org/apache/ratis/statemachine/TransactionContext.java:
##########
@@ -94,11 +95,40 @@ public interface TransactionContext {
LogEntryProto initLogEntry(long term, long index);
/**
- * Returns the committed log entry
- * @return the committed log entry
+ * @return a copy of the committed log entry if it exists; otherwise,
returns null
+ *
+ * @deprecated Use {@link #getLogEntryRef()} or {@link #getLogEntryUnsafe()}
to avoid copying.
*/
+ @Deprecated
LogEntryProto getLogEntry();
+ /**
+ * @return the committed log entry if it exists; otherwise, returns null.
+ * The returned value is safe to use only before {@link
StateMachine#applyTransaction} returns.
+ * Once {@link StateMachine#applyTransaction} has returned, it is
unsafe to use the log entry
+ * since the underlying buffers can possiby be released.
+ */
+ default LogEntryProto getLogEntryUnsafe() {
+ return getLogEntryRef().get();
+ }
+
+ /**
+ * Get a {@link ReferenceCountedObject} to the committed log entry.
+ *
+ * It is safe to access the log entry by calling {@link
ReferenceCountedObject#get()}
+ * (without {@link ReferenceCountedObject#retain()})
+ * inside the scope of {@link StateMachine#applyTransaction}.
+ *
+ * If the log entry is needed after {@link StateMachine#applyTransaction}
returns,
+ * e.g. for asynchronous computation or caching,
+ * the caller must invoke {@link ReferenceCountedObject#retain()} and {@link
ReferenceCountedObject#release()}.
+ *
+ * @return a reference to the committed log entry if it exists; otherwise,
returns null.
+ */
+ default ReferenceCountedObject<LogEntryProto> getLogEntryRef() {
+ return
Optional.ofNullable(getLogEntryUnsafe()).map(this::wrap).orElse(null);
+ }
+
Review Comment:
It is better to use `getLogEntryUnsafe()` in the `wrap(..)` method below.
```
final LogEntryProto expected = getLogEntryUnsafe();
Objects.requireNonNull(expected, "logEntry == null");
Preconditions.assertSame(expected.getTerm(), entry.getTerm(),
"entry.term");
Preconditions.assertSame(expected.getIndex(), entry.getIndex(),
"entry.index");
```
##########
ratis-server-api/src/main/java/org/apache/ratis/statemachine/TransactionContext.java:
##########
@@ -94,11 +95,40 @@ public interface TransactionContext {
LogEntryProto initLogEntry(long term, long index);
/**
- * Returns the committed log entry
- * @return the committed log entry
+ * @return a copy of the committed log entry if it exists; otherwise,
returns null
+ *
+ * @deprecated Use {@link #getLogEntryRef()} or {@link #getLogEntryUnsafe()}
to avoid copying.
*/
+ @Deprecated
LogEntryProto getLogEntry();
+ /**
+ * @return the committed log entry if it exists; otherwise, returns null.
+ * The returned value is safe to use only before {@link
StateMachine#applyTransaction} returns.
+ * Once {@link StateMachine#applyTransaction} has returned, it is
unsafe to use the log entry
+ * since the underlying buffers can possiby be released.
Review Comment:
@duongkame , this typo is not fixed yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
