Skip to content
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 burst only #396

Closed
wants to merge 22 commits into from
Closed

Fix burst only #396

wants to merge 22 commits into from

Conversation

toad
Copy link
Contributor

@toad toad commented Aug 12, 2015

Various cleanups, mostly directed at fixing logging errors. Relatively low risk but could conceivably change connectivity behaviour on non-NATed nodes.

toad added 7 commits August 12, 2015 21:10
This should fix some annoying log spam and rekey quicker.
Should fix bogus logging on nodes with darknet peers.
This fixes logger errors due to trying to reconnect to old opennet peers
which are not in the routing table and not in the status table either, but
get set to BURSTING on non-NATed nodes.
toad added 8 commits August 14, 2015 17:09
Mainly for squashing log errors, but the peer counts *are* used for important
things, notably determining whether we need more opennet peers.
Conflicts:
	src/freenet/node/PeerNode.java
This reverts commit f98ac58.

This is wrong. There is a better way...

Conflicts:
	src/freenet/node/PeerNode.java
This makes much more sense. It also fixes the "old opennet peers in the peer
status tracker" problem more cleanly, and removes some unnecessary argument
cruft. (Renames forceCancelDisconnecting to onAdded and always calls it in
PeerManager.addPeer)
@toad
Copy link
Contributor Author

toad commented Aug 15, 2015

I've added a bunch more fixes, I think this is reasonable now. Please re-review. :)

@toad
Copy link
Contributor Author

toad commented Aug 15, 2015

Yes, DarknetPeerNode's setStatus method calls PeerNode's setStatus and then changes the value in some cases. I will add some javadocs.

@toad
Copy link
Contributor Author

toad commented Aug 15, 2015

I'll try not to add more unrelated bugfixes to this branch! Please review. Thanks.

@Thynix
Copy link
Contributor

Thynix commented Oct 11, 2015

Anyone want to review this?

@ArneBab
Copy link
Contributor

ArneBab commented Nov 10, 2015

@toad could you explain what the non-logging changes here do?

@nextgens nextgens added the bugfix label Jan 4, 2016
@nextgens nextgens added this to the build1472 milestone Jan 4, 2016
@nextgens
Copy link
Contributor

nextgens commented Jan 4, 2016

Looks good to me

@Thynix
Copy link
Contributor

Thynix commented May 14, 2016

Can I trouble someone to resolve the merge conflicts? I attempted fixing them but it wasn't exceedingly clear to me what was intended.

@nextgens nextgens removed this from the build1472 milestone Jun 26, 2016
@Thynix
Copy link
Contributor

Thynix commented Jul 18, 2016

Ping on those merge conflicts.

@Thynix
Copy link
Contributor

Thynix commented Sep 15, 2016

Cough cough.

@ArneBab
Copy link
Contributor

ArneBab commented Feb 20, 2017

ping on the merge conflicts from my side, too

@ArneBab
Copy link
Contributor

ArneBab commented Apr 29, 2019

ping?

@ArneBab ArneBab added the mergeAfterResolvingConflicts there are conflicts but code review resolution was "merge" label Jun 18, 2022
@ArneBab
Copy link
Contributor

ArneBab commented Jun 18, 2022

@toad ping on the merge conflicts again

@ArneBab
Copy link
Contributor

ArneBab commented Nov 11, 2024

Here’s a version with fixed merge errors. It needs re-review, though, because the code it patches changed quite a bit since 2016: #991

@ArneBab
Copy link
Contributor

ArneBab commented Nov 11, 2024

replaced by #991

@ArneBab ArneBab closed this Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix mergeAfterResolvingConflicts there are conflicts but code review resolution was "merge"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants