RATIS-2508. appendEntries log messages improvement.#1440
RATIS-2508. appendEntries log messages improvement.#1440szetszwo wants to merge 3 commits intoapache:masterfrom
Conversation
adoroszlai
left a comment
adoroszlai
left a comment
There was a problem hiding this comment.
Thanks @szetszwo for the patch.
| : n == 1 ? "entry" + first | ||
| : n + " entries:" + first + "..." + last; |
There was a problem hiding this comment.
Missing :?
| : n == 1 ? "entry" + first | |
| : n + " entries:" + first + "..." + last; | |
| : n == 1 ? "entry:" + first | |
| : n + " entries:" + first + "..." + last; |
Also, would = instead of : be better? Since AppendEntriesRequest#toString uses : as separator.
| LOG.warn("{}: appendEntries* failed: {}", getMemberId(), toLogEntryTermIndexString(entries), t); | ||
| } else if (LOG.isDebugEnabled()) { | ||
| LOG.debug("{}: appendEntries* succeeded {}", getMemberId(), toLogEntryTermIndexString(entries)); |
There was a problem hiding this comment.
nit: failed: {} has : but succeeded {} does not.
|
@adoroszlai , thanks for reviewing this! Just pushed a change to address your comments. |
adoroszlai
left a comment
adoroszlai
left a comment
There was a problem hiding this comment.
Thanks @szetszwo for updating the patch, LGTM.
OneSizeFitsQuorum
left a comment
OneSizeFitsQuorum
left a comment
There was a problem hiding this comment.
LGTM~ Only a minor problem
| if (roleChangeChecking(electionTimeout)) { | ||
| LOG.info("{}: change to CANDIDATE, lastRpcElapsedTime:{}, electionTimeout:{}", | ||
| this, lastRpcTime.elapsedTime(), electionTimeout); | ||
| LOG.info("{}: change to CANDIDATE, lastRpcElapsedTime:{}ms, electionTimeout:{}", |
There was a problem hiding this comment.
why we add ms for lastRpcElapsedTime but not electionTimeout
There was a problem hiding this comment.
Before this change
- electionTimeout is a TimeDuration in ms
- elapsedTime() returns a TimeDuration in ns.
So, I changed elapsedTime() to elapsedTimeMs() and added ms.
3 participants
See RATIS-2508
Due to checkstyle line number limit (2000), this PR also refactors RaftServerImpl.