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

Remove code related to NLM load sending #1002

Merged
merged 1 commit into from
Nov 30, 2024

Conversation

bertm
Copy link
Contributor

@bertm bertm commented Nov 17, 2024

This code was effectively inactive since 2013 (which is when the FIXME in PeerNode.makeLoadStats was added: "re-enable when try NLM again").

Removing the related code not only gets rid of a few hundred lines of code, but also slightly reduces the (huge) number of things PeerNode needs to keep track of and simplifies the NPF logic substantially.

Note: this does not remove receiving of load stats from peer nodes.

@bertm bertm force-pushed the remove-nlm-load-sending branch from 909e0dc to 6fcf4f0 Compare November 17, 2024 18:19
@ArneBab
Copy link
Contributor

ArneBab commented Nov 22, 2024

Thank you for the many explanatory comments in message handling parts (including the concurrency warnings). These are critical and comments there should easy maintenance a lot!

@bertm
Copy link
Contributor Author

bertm commented Nov 30, 2024

I'm not aware of adding any explanatory comments in the message handling parts; those are probably attributed to me due to indentation changes (a few outer while(true) loops were removed).

This code was effectively inactive since 2013 (which is when the FIXME
in PeerNode.makeLoadStats was added: "re-enable when try NLM again").

Removing the related code not only gets rid of a few hundred lines of
code, but also slightly reduces the (huge) number of things PeerNode
needs to keep track of and simplifies the NPF logic substantially.

Note: this does not remove receiving of load stats from peer nodes.
@bertm bertm force-pushed the remove-nlm-load-sending branch from 6fcf4f0 to f7fa0c4 Compare November 30, 2024 18:44
@bertm
Copy link
Contributor Author

bertm commented Nov 30, 2024

Rebased on next to resolve merge conflicts in NewPacketFormatTest.

@ArneBab ArneBab merged commit 25916aa into hyphanet:next Nov 30, 2024
1 check passed
@ArneBab
Copy link
Contributor

ArneBab commented Nov 30, 2024

I now reviewed directly in Emacs instead of on github and there (with ignore whitespace) this was clear to review. Merged — thank you!

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