-
Notifications
You must be signed in to change notification settings - Fork 214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: only log errors when the time was invalid in 1497, too #1035
base: next
Are you sure you want to change the base?
Fix: only log errors when the time was invalid in 1497, too #1035
Conversation
Creating backtraces isn’t free and a user reported that they see a lot of these errors in the log. But the behavior is expected if you’re connected to a node with build 1497 or earlier.
src/freenet/node/FailureTable.java
Outdated
@@ -115,12 +117,15 @@ public void start() { | |||
*/ | |||
public void onFailed(Key key, PeerNode routedTo, short htl, long rfTimeout, long ftTimeout) { | |||
if(ftTimeout < 0 || ftTimeout > REJECT_TIME) { | |||
Logger.error(this, "Bogus timeout "+ftTimeout, new Exception("error")); | |||
if (ftTimeout > 0 && rfTimeout > REJECT_TIME_BEFORE_BUILD_1498) { // only log an error if the time is invalid for 1497, too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This checks both ftTimeout
and rfTimeout
, is that okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that’s not ok … fixed.
src/freenet/node/FailureTable.java
Outdated
ftTimeout = Math.max(Math.min(REJECT_TIME, ftTimeout), 0); | ||
} | ||
if(rfTimeout < 0 || rfTimeout > RECENTLY_FAILED_TIME) { | ||
if(rfTimeout > 0) | ||
if(rfTimeout > 0 && rfTimeout > RECENTLY_FAILED_TIME_BEFORE_BUILD_1498) { // only log an error if the time is invalid for 1497, too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If rfTimeout
is largen than RECENTLY_FAILED_TIME_BEFORE_BUILD_1498
, it should also be larger than 0
, should it not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it should — two unnecessary comparisons removed. Thank you!
Thanks to Bombe!
Creating backtraces isn’t free and a user reported that they see a lot of these errors in the log. But the behavior is expected if you’re connected to a node with build 1497 or earlier.