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

Kademlia load balancing #28

Closed
wants to merge 2 commits into from

Conversation

nolash
Copy link
Contributor

@nolash nolash commented Sep 14, 2019

in progress


* Given `m` requests to `n` peers in a bin, after `m*n` requests the peers will have made `m` requests each.
* Given `m` requests to `n` peers within the neighborhood, after `m*n` requests the peers will have made `m` requests each.
* Given `m` requests for no capability and `ma` requests for capability `a` to peers `n` with all capabilities and peers `na` with capability `a`, after `n*n + ma*na` requests the `ma` peers will have made `ma+m` requests each.
Copy link

@kortatu kortatu Sep 16, 2019

Choose a reason for hiding this comment

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

n * n -> m * n
the ma -> the na

I think it should be.

Suggested change
* Given `m` requests for no capability and `ma` requests for capability `a` to peers `n` with all capabilities and peers `na` with capability `a`, after `n*n + ma*na` requests the `ma` peers will have made `ma+m` requests each.
* Given `m` requests for no capability and `ma` requests for capability `a` to peers `n` with all capabilities and peers `na` with capability `a`, after `m*n + ma*na` requests the `na` peers will have made `ma+m` requests each.


## Specification

The component should be a layer above the kademlia itself, and most likely sits in the same layer as - or is even perhaps the same component as - the planned convergence of the request and message forwarding engine (todo link to related issue).
Copy link

Choose a reason for hiding this comment

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

Forwarding issue:
ethersphere/swarm#1656

@kortatu
Copy link

kortatu commented Sep 18, 2019

What is the definition of a request? Should be accounted each time a peer is returned to the caller or it depends on the use that caller is doing with the Peer.
For example, in Pss, not all peers retrieved with EachConn are used:

p.Kademlia.EachConn(to, addressLength*8, func(sp *network.Peer, po int) bool {
		if po < broadcastThreshold && sent > 0 {
			return false // stop iterating
		}
...

In this case, sp won't be used.
I can see here two possibilities:

  1. Each time a peer is used to execute the f function passed to EachConn is accounted as one request.

  2. The caller to the load balancing service must signal in some way that a Peer has been used (complicating the API of that service) to be accounted as one request.

@nolash
Copy link
Contributor Author

nolash commented Sep 18, 2019

Number 2.

A bool argument should suffice, I guess? We can rename the function, then have a new function with the existing name wrap that function, with that argument set so accounting is not done.

@kortatu
Copy link

kortatu commented Sep 18, 2019

Number 2.

A bool argument should suffice, I guess? We can rename the function, then have a new function with the existing name wrap that function, with that argument set so accounting is not done.

I was implementing something like that, but is a returning value of that f function (now returns two values, one to continue iterating one to signaling that the peer has been used)


## Motivation

Currently no logic exists to guarantee that routing is distributed among peers in a bin. If a bin is unchanged between kademlia queries for forwarding peers in that bin, currently the most likely result is that the same peer will always be returned. This
Copy link
Member

Choose a reason for hiding this comment

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

"returned. This
Hence ... "
can you please reword this?

@crtahlin crtahlin added the check-SWIP-status Check if the SWIP is still relevant and being pursued. label Jun 20, 2023
@crtahlin crtahlin closed this Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
check-SWIP-status Check if the SWIP is still relevant and being pursued.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants