-
Notifications
You must be signed in to change notification settings - Fork 251
feat: rework fullrt to use a caching routing table #1084
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
base: master
Are you sure you want to change the base?
Conversation
// getRange takes the closest keys to kadKey and returns two keys representative of the lower and upper bounds | ||
func getRange(closestKeys []kadkey.Key, kadKey kadkey.Key) (kadkey.Key, kadkey.Key) { | ||
slices.SortFunc(closestKeys, func(a, b kadkey.Key) int { | ||
return bytes.Compare(a, b) | ||
}) | ||
var k, lowerKeyScratch, higherKeyScratch big.Int | ||
k.SetBytes(kadKey) | ||
lowerKeyScratch.SetBytes(closestKeys[0]) | ||
lowerKeyScratch.Sub(&k, &lowerKeyScratch) // lowerKeyScratch = key - lowestKey | ||
higherKeyScratch.SetBytes(closestKeys[len(closestKeys)-1]) | ||
higherKeyScratch.Sub(&k, &higherKeyScratch) // higherKeyScratch = key - highestKey | ||
|
||
var lowest, highest kadkey.Key | ||
if lowerKeyScratch.Sign() == -1 { | ||
// lowestKey is still higher than the key | ||
// So higher = highestKey, lower = key - (highestKey - key) | ||
lowerKeyScratch.Add(&k, &higherKeyScratch) | ||
lowestBytes := make([]byte, 32) | ||
if lowerKeyScratch.Sign() == 1 { | ||
lowest = lowerKeyScratch.FillBytes(lowestBytes) | ||
} else { | ||
lowest = lowestBytes | ||
} | ||
highest = closestKeys[len(closestKeys)-1] | ||
} else if higherKeyScratch.Sign() == 1 { | ||
// highestKey is still lower than the key | ||
// So lower = lowestKey, higher = key + (key - lowestKey) | ||
higherKeyScratch.Add(&k, &lowerKeyScratch) | ||
highestBytes := make([]byte, 32) | ||
if higherKeyScratch.Cmp(max32Bytes) >= 0 { | ||
highest = max32Bytes.FillBytes(highestBytes) | ||
} else { | ||
highest = higherKeyScratch.FillBytes(highestBytes) | ||
} | ||
lowest = closestKeys[0] | ||
} else { // Handle 0? | ||
switch higherKeyScratch.CmpAbs(&lowerKeyScratch) { | ||
case -1: | ||
// highest - key < key - lowest | ||
// So lower = lowestKey, higher = key + (key - lowestKey) | ||
lowest = closestKeys[0] | ||
higherKeyScratch.Add(&k, &lowerKeyScratch) | ||
highestBytes := make([]byte, 32) | ||
if higherKeyScratch.Cmp(max32Bytes) >= 0 { | ||
highest = max32Bytes.FillBytes(highestBytes) | ||
} else { | ||
highest = higherKeyScratch.FillBytes(highestBytes) | ||
} | ||
case 1: | ||
// highest - key > key - lowest | ||
// So higher = highestKey, lower = key - (highestKey - key) | ||
highest = closestKeys[len(closestKeys)-1] | ||
lowerKeyScratch.Add(&k, &higherKeyScratch) | ||
lowestBytes := make([]byte, 32) | ||
if lowerKeyScratch.Sign() == 1 { | ||
lowest = lowerKeyScratch.FillBytes(lowestBytes) | ||
} else { | ||
lowest = lowestBytes | ||
} | ||
case 0: | ||
// TODO: edge case with one peer | ||
lowest = closestKeys[0] | ||
highest = closestKeys[len(closestKeys)-1] | ||
} | ||
} | ||
|
||
return lowest, highest | ||
} |
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.
IIUC correctly this logic doesn't work because the XOR keyspace isn't linear. I'll try to demonstrate with an example.
Let's consider a 4-bit keyspace. GetClosestPeers is performed to find the 5 closest peers to 0b1010
(10
in decimal). The five closest peers in XOR distance are, in order 10
(self), 11
, 8
, 9
and 14
.
key = 10
lowestKey = 8
highestKey = 14
lowerKeyScratch = key - lowestKey
higherKeyScratch = key - highestKey
lowerKeyScratch = 10
- 8
= 2
higherKeyScratch = 10
- 14
= -4
-> enter the else
at line 584 since lowerKeyScratch
is pos and higherKeyScratch
is neg.
higherKeyScratch.CmpAbs(&lowerKeyScratch) => 1 (highest - key > key - lowest)
higher = highestKey, lower = key - (highestKey - key)
higher = 14
lower = 10
- (14
- 10
) = 6
So the returned covered range is expected to be from 6
to 14
, which doesn't work since the following keys are in the range and not covered 6
, 7
, 12
, 13
.
I don't think we can use additions and subtractions in the XOR keyspace, we are limited to the XOR operation to compute distances.
The method I am using in #1082 to determine which range (branch of the binary trie) of the keyspace is covered is as follow:
Input: key
, closest_peers
Compute Common Prefix Length (cpl
) of all closest_peers
(or min(cpl(key, peer) for all peers in closest_peers)
, which is equivalent).
Output: key[:cpl+1]
It means that lowest would be key[:cpl+1]+"00...0"
and highest key[:cpl+1]+"11...1"
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.
Oy well that's embarrassing 😅. I'm not sure why I used linear space here.
It looks like we could might be able to keep a bunch of the structure here if we changed how the comparators work to leverage XOR space correctly, but is it worth keeping almost any of the range caching logic given that the way you're doing the ranges simplifies to basically just saying which subtrees/prefixes are covered which could be handled / optimized differently?
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, we could probably use a binary trie to simplify coverage tracking 👍🏻
cc @guillaumemichel this should be complimentary or may help with #1082. It operates by reworking the fullrt client to not actually be a full routing table, but have a caching routing table.
Feel free to change/rewrite anything here 😅.