szetszwo commented on code in PR #1177:
URL: https://github.com/apache/ratis/pull/1177#discussion_r1839082768
##########
ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderBase.java:
##########
@@ -261,12 +261,19 @@ protected
ReferenceCountedObject<AppendEntriesRequestProto> nextAppendEntriesReq
final long halfMs = heartbeatWaitTimeMs/2;
final Map<Long, ReferenceCountedObject<EntryWithData>> offered = new
HashMap<>();
for (long next = followerNext; leaderNext > next &&
getHeartbeatWaitTimeMs() - halfMs > 0; next++) {
- final ReferenceCountedObject<EntryWithData> entryWithData =
getRaftLog().retainEntryWithData(next);
- if (!buffer.offer(entryWithData.get())) {
- entryWithData.release();
- break;
+ ReferenceCountedObject<EntryWithData> entryWithData = null;
+ try {
+ entryWithData = getRaftLog().retainEntryWithData(next);
+ if (!buffer.offer(entryWithData.get())) {
+ entryWithData.release();
+ break;
+ }
+ offered.put(next, entryWithData);
+ } catch (Exception e){
+ if (entryWithData != null) {
+ entryWithData.release();
+ }
Review Comment:
```java
LogAppenderDaemon failed
org.apache.ratis.server.raftlog.RaftLogIOException: Log entry not found:
index = 4269
at
org.apache.ratis.server.raftlog.segmented.SegmentedRaftLog.retainEntryWithData(SegmentedRaftLog.java:334)
at
org.apache.ratis.server.leader.LogAppenderBase.nextAppendEntriesRequest(LogAppenderBase.java:264)
```
For the above particular exception, this change won't help since, when
`retainEntryWithData(..)` throws an exception, `entryWithData` must be null.
This change will help if other methods (e.g. `get()`, `offer(..)`,
`put(..)`) throw an exception. However, these methods throw only runtime
exceptions/errors (e.g. OutOfMemoryError). We may not need to handle it.
##########
pom.xml:
##########
@@ -643,7 +643,7 @@
<enableProcessChecker>all</enableProcessChecker>
<forkedProcessTimeoutInSeconds>600</forkedProcessTimeoutInSeconds>
<!-- @argLine is filled by jacoco maven plugin. @{} means late
evaluation -->
- <argLine>-Xmx2g -XX:+HeapDumpOnOutOfMemoryError
@{argLine}</argLine>
+ <argLine>-Xmx8g -XX:+HeapDumpOnOutOfMemoryError
@{argLine}</argLine>
Review Comment:
When turning on the advanced reference trace, it does need more memory. I
recall that I did similar change for running with advanced reference trace.
##########
pom.xml:
##########
@@ -583,7 +583,7 @@
<configuration>
<fork>true</fork>
<meminitial>512m</meminitial>
- <maxmem>2048m</maxmem>
+ <maxmem>4096m</maxmem>
Review Comment:
Are you sure that it needs more memory for compilation?
--
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]
