szetszwo commented on code in PR #1074:
URL: https://github.com/apache/ratis/pull/1074#discussion_r1588010038
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLogBase.java:
##########
@@ -83,10 +85,12 @@ public abstract class RaftLogBase implements RaftLog {
private final long purgePreservation;
private final AtomicReference<LogEntryProto> lastMetadataEntry = new
AtomicReference<>();
+ private final Function<StateMachineLogEntryProto, String>
stateMachineToString;
Review Comment:
Let don't add `StateMachine` dependency `RaftLogBase` . The subclasses can
override `toLogEntryString(..)`.
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcServerProtocolService.java:
##########
@@ -316,7 +316,7 @@ boolean isHeartbeat(AppendEntriesRequestProto request) {
@Override
String requestToString(AppendEntriesRequestProto request) {
- return ServerStringUtils.toAppendEntriesRequestString(request);
+ return ServerStringUtils.toAppendEntriesRequestString(request, e ->
"");
Review Comment:
Let's pass `null`. The code currently can handle `null`.
BTW, could you also update `stateMachineLogEntryProtoToString(..)` to print
only sizes and
`toLogEntryString(..)` to use `stateMachineLogEntryProtoToString(..)`?
```java
@@ -70,7 +70,11 @@ public final class LogProtoUtils {
}
static String stateMachineLogEntryProtoToString(StateMachineLogEntryProto
p) {
- return "logData:" + p.getLogData() + ", stateMachineEntry:" +
p.getType() + ":" + p.getStateMachineEntry();
+ final StateMachineEntryProto stateMachineEntry =
p.getStateMachineEntry();
+ return p.getType()
+ + ": logData.size=" + p.getLogData().size()
+ + ", stateMachineData.size=" +
stateMachineEntry.getStateMachineData().size()
+ + ", logEntryProtoSerializedSize=" +
stateMachineEntry.getLogEntryProtoSerializedSize();
}
```
```java
@@ -46,9 +45,10 @@ public final class LogProtoUtils {
}
final String s;
if (entry.hasStateMachineLogEntry()) {
- s = ", " + Optional.ofNullable(function)
- .orElseGet(() -> proto -> "" + ClientInvocationId.valueOf(proto))
- .apply(entry.getStateMachineLogEntry());
+ if (function == null) {
+ function = LogProtoUtils::stateMachineLogEntryProtoToString;
+ }
+ s = ", " + function.apply(entry.getStateMachineLogEntry());
} else if (entry.hasMetadataEntry()) {
final MetadataProto metadata = entry.getMetadataEntry();
s = "(c:" + metadata.getCommitIndex() + ")";
```
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLogBase.java:
##########
@@ -199,10 +208,10 @@ private long appendImpl(long term, TransactionContext
operation) throws StateMac
appendEntry(operation.wrap(e), operation).whenComplete((returned, t) -> {
if (t != null) {
- LOG.error(name + ": Failed to write log entry " +
LogProtoUtils.toLogEntryString(e), t);
+ LOG.error(name + ": Failed to write log entry " + toString(e), t);
} else if (returned != nextIndex) {
LOG.error("{}: Indices mismatched: returned index={} but
nextIndex={} for log entry {}",
- name, returned, nextIndex, LogProtoUtils.toLogEntryString(e));
+ name, returned, nextIndex, toString(e));
Review Comment:
Use `this.toLogEntryString(..)`.
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLogBase.java:
##########
@@ -532,11 +541,8 @@ private void discardData() {
@Override
public String toString() {
- return toLogEntryString(logEntry);
+ return RaftLogBase.this.toString(logEntry);
}
}
- public String toLogEntryString(LogEntryProto logEntry) {
- return LogProtoUtils.toLogEntryString(logEntry);
- }
Review Comment:
Let's keep this method.
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLog.java:
##########
@@ -467,7 +468,7 @@ protected CompletableFuture<Long>
appendEntryImpl(ReferenceCountedObject<LogEntr
}
return write.getFuture().whenComplete((clientReply, exception) ->
appendEntryTimerContext.stop());
} catch (Exception e) {
- LOG.error("{}: Failed to append {}", getName(),
LogProtoUtils.toLogEntryString(entry), e);
+ LOG.error("{}: Failed to append {}", getName(), toString(entry), e);
Review Comment:
Use `this.toLogEntryString(..)`.
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLog.java:
##########
@@ -345,8 +347,7 @@ public ReferenceCountedObject<EntryWithData>
retainEntryWithData(long index) thr
}
return future != null? newEntryWithData(entryRef, future):
newEntryWithData(entryRef);
} catch (Exception e) {
- final String err = getName() + ": Failed readStateMachineData for " +
- LogProtoUtils.toLogEntryString(entry);
+ final String err = getName() + ": Failed readStateMachineData for " +
toString(entry);
Review Comment:
Use `this.toLogEntryString(..)`.
--
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]
