Skip to content

IGNITE-24643 Lower logging level for 'Peer id not found' in Replicator#5299

Merged
rpuch merged 4 commits intoapache:mainfrom
gridgain:ignite-24643
Feb 26, 2025
Merged

IGNITE-24643 Lower logging level for 'Peer id not found' in Replicator#5299
rpuch merged 4 commits intoapache:mainfrom
gridgain:ignite-24643

Conversation

@rpuch
Copy link
Contributor

@rpuch rpuch commented Feb 26, 2025

}
}

private static void logFailToIssueRpc(Status status, Replicator r) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's name it replicator :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.S. quite to be honest PMD rule to prevent single symbol names is a good one and we shall consider enable it ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is JRaft code (not ours), we try to touch it as little as possible, so the checks (checkstyle/PMD/whatever) are disabled in this module

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, thanks.


private static void logFailToIssueRpc(Status status, Replicator r) {
if (status.getRaftError() == RaftError.ENOENT) {
// Maybe the target node was not able to start yet, no need to WARN here.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move it to javaDoc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt it's needed there. Javadoc is about method interface, but here it's an implementation detail

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can contain implementation details, especially for private methods.
At least at will be nicer to read what is going on and we need to branch.

Comment on lines 1248 to 1249
LOG.info("Fail to issue RPC to {}, consecutiveErrorTimes={}, error={}", peerId,
consecutiveErrorTimes, status);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
LOG.info("Fail to issue RPC to {}, consecutiveErrorTimes={}, error={}", peerId,
consecutiveErrorTimes, status);
LOG.info("Fail to issue RPC to {}, consecutiveErrorTimes={}, error={}", peerId, consecutiveErrorTimes, status);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it nicely fits into one line ;)

Comment on lines 1251 to 1252
LOG.warn("Fail to issue RPC to {}, consecutiveErrorTimes={}, error={}", peerId,
consecutiveErrorTimes, status);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
LOG.warn("Fail to issue RPC to {}, consecutiveErrorTimes={}, error={}", peerId,
consecutiveErrorTimes, status);
LOG.warn("Fail to issue RPC to {}, consecutiveErrorTimes={}, error={}", peerId, consecutiveErrorTimes, status);

@rpuch rpuch merged commit dac2815 into apache:main Feb 26, 2025
1 check passed
@rpuch rpuch deleted the ignite-24643 branch February 26, 2025 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants