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.

Sign up for GitHub

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

Jump to bottom

Seednode optimizations #2501

Merged
merged 3 commits into from
Jul 8, 2019
Merged

Seednode optimizations #2501

merged 3 commits into from
Jul 8, 2019

Conversation

freimair
Copy link
Contributor

@freimair freimair commented Mar 6, 2019

No description provided.

@freimair freimair mentioned this pull request Mar 6, 2019
Closed
@ManfredKarrer ManfredKarrer self-requested a review March 6, 2019 15:15
ManfredKarrer
ManfredKarrer approved these changes Mar 6, 2019
View reviewed changes
Copy link
Contributor

@ManfredKarrer ManfredKarrer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

ACK EDIT: Did not see it was a draft...

p2p/src/main/java/bisq/network/p2p/P2PService.java Outdated Show resolved Hide resolved
@ManfredKarrer
Copy link
Contributor

Ah sorry just saw it is in draft.... So the ACk is for current state ;-)

@ManfredKarrer ManfredKarrer self-requested a review March 7, 2019 03:59
@freimair freimair force-pushed the seednode_optimizations branch from 8223148 to 27dd8c2 Compare March 28, 2019 09:45
@freimair freimair marked this pull request as ready for review March 28, 2019 14:43
@freimair
Copy link
Contributor Author

had the jhgcy2won7xnslrb.onion seed node running with this change since 2019-03-28 09:54 UTC. No issues showed up. yet.

The issue has been that, at times, data reached the seed nodes later than an active client. So whenever the client requested data from the seed node, it happened that the seed node delivered data which the client already removed due to a remove-message. As of now, we keep a (limited) history of messages (i.e. their hashes) we already removed and check if we already removed a message we get.

ManfredKarrer
ManfredKarrer reviewed Mar 29, 2019
View reviewed changes
Copy link
Contributor

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

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

I think we cannot do that. If the user removes and offer and then enables it again it would not be added until the 500 items get cleared out.
Have you found out why the seed node gets delayed some messages? Might be overall too heavy load of seed or that some ddos protection causes that?

@freimair
Copy link
Contributor Author

Well, I thought that removing and offer and republishing an offer results in a fresh hash...

ad delayed messages: not only seed nodes are affected. the issue arises whenever one node (which gets asked for UpdateData) has not yet received a remove command but the the asking node has. A few ms are enough to cause the issue. Has nothing to do with heavy load or any DoS protection - just plain P2P network latency.

If there is no fresh hash for a fresh message, then we have these options:

  • revert to making seed nodes crash and restart again
  • reduce the 500 to say 5 minutes?
  • only apply the filter to UpdateDataResponses

@freimair freimair force-pushed the seednode_optimizations branch from 8a07e10 to 956931d Compare April 1, 2019 10:03
@ManfredKarrer ManfredKarrer changed the title Seednode optimizations [WIP] Seednode optimizations Apr 11, 2019
ManfredKarrer
ManfredKarrer reviewed May 14, 2019
View reviewed changes
Copy link
Contributor

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

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

Requires another review. Previous ACK was a mistake.

@freimair freimair force-pushed the seednode_optimizations branch from 956931d to 06d2186 Compare July 5, 2019 07:21
@freimair freimair requested a review from ripcurlx as a code owner July 5, 2019 07:21
@freimair
Copy link
Contributor Author

freimair commented Jul 5, 2019

I just saw that this PR is still open so I

  • made it work with the current master
  • removed the bits with the repeated UpdateDataRequests, as this seems not to work so well

@freimair freimair changed the title [WIP] Seednode optimizations Seednode optimizations Jul 5, 2019
@freimair freimair mentioned this pull request Jul 8, 2019
Closed
ripcurlx
ripcurlx approved these changes Jul 8, 2019
View reviewed changes
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK

@ripcurlx ripcurlx merged commit 6490c3d into bisq-network:master Jul 8, 2019
@devinbileck devinbileck mentioned this pull request Jul 8, 2019
Open
freimair added a commit to freimair/bisq-docs that referenced this pull request Jul 20, 2019
@freimair
Added systemd service howto
01b1cc1 
systemd support has been added in bisq-network/bisq#2501
@freimair freimair deleted the seednode_optimizations branch December 6, 2019 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrosseel mrosseel mrosseel left review comments

@ManfredKarrer ManfredKarrer ManfredKarrer approved these changes

@ripcurlx ripcurlx ripcurlx approved these changes

Assignees

@freimair freimair

Labels
in:network
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@freimair @ManfredKarrer @ripcurlx @mrosseel