RATIS-2184. Improve TestRaftWithGrpc test stability #1177
Conversation
|
@szetszwo @duongkame , can you help take it a look? |
There was a problem hiding this comment.
@jianghuazhu , thanks a lot for working on this! Please see the comments inlined.
| 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(); | ||
| } |
There was a problem hiding this comment.
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.
| @@ -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> | |||
There was a problem hiding this 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.
| <maxmem>2048m</maxmem> | ||
| <maxmem>4096m</maxmem> |
There was a problem hiding this comment.
Are you sure that it needs more memory for compilation?
What changes were proposed in this pull request?
Improve stability of TestRaftWithGrpc tests.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-2184
How was this patch tested?
ci:
https://github.com/jianghuazhu/ratis/actions/runs/11792452996