Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Cleanup ibft logging levels #995

Merged
merged 14 commits into from
Feb 28, 2019

Conversation

jframe
Copy link
Contributor

@jframe jframe commented Feb 27, 2019

PR description

Updating IBFT logging levels to make more consistent.
Debug level is now predominately about the state machine send, receives and quorums.
Info now also includes validation failures so that is visible.
Trace now alsol includes additional receives in parts of the state machine

Fixed Issue(s)

extraData,
block.getHeader());

LOG.debug("Creating proposed block. round={}", roundState.getRoundIdentifier());
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: tempted to put a trace in here too with the additional info?

Copy link
Contributor Author

@jframe jframe Feb 28, 2019

Choose a reason for hiding this comment

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

done. Didn't think it was useful at debug level and it made the logs difficult to read. but at trace it's probably appropriate.

@@ -94,7 +94,7 @@ public RoundChangeManager(
public Optional<Collection<RoundChange>> appendRoundChangeMessage(final RoundChange msg) {

if (!isMessageValid(msg)) {
LOG.info("RoundChange message was invalid.");
LOG.debug("RoundChange message was invalid.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is an invalid RoundChange debug, but an invalid Proposal an info? (see IbftBlockHeightManager) - I think I prefer info in both places (this is an exceptional behaviour and we want to see it even normally)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -80,15 +80,13 @@ public boolean setProposedBlock(final Proposal msg) {
public void addPrepareMessage(final Prepare msg) {
if (!proposalMessage.isPresent() || validator.validatePrepare(msg)) {
prepareMessages.add(msg);
LOG.debug("Round state added prepare message prepare={}", msg);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: not even trace? ... maybe not ... (just want to confirm!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. made it trace

@rain-on rain-on merged commit 07446b1 into PegaSysEng:master Feb 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants