szetszwo commented on code in PR #1042:
URL: https://github.com/apache/ratis/pull/1042#discussion_r1484667304
##########
ratis-server-api/src/main/java/org/apache/ratis/statemachine/StateMachine.java:
##########
@@ -88,6 +88,11 @@ default CompletableFuture<ByteString> read(LogEntryProto
entry, TransactionConte
return read(entry);
}
+ default CompletableFuture<ReferenceCountedObject<ByteString>>
readWithRef(LogEntryProto entry,
Review Comment:
Why this new method is not used anywhere?
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/memory/MemoryRaftLog.java:
##########
@@ -108,16 +108,17 @@ public RaftLogMetricsBase getRaftLogMetrics() {
}
@Override
- public LogEntryProto get(long index) {
+ public ReferenceCountedObject<LogEntryProto> getWithRef(long index) {
checkLogState();
- try(AutoCloseableLock readLock = readLock()) {
+ try (AutoCloseableLock readLock = readLock()) {
return entries.get(Math.toIntExact(index));
}
}
@Override
public EntryWithData getEntryWithData(long index) {
- return newEntryWithData(get(index), null);
+ // TODO. The reference counted object should be passed to LogAppender
RATIS-2026.
Review Comment:
Does it make more sense to move the `RaftLog` changes to RATIS-2026?
##########
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:
Sorry, there was a typo in my review: possiby -> possibly
--
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]
