feat: allow setting Kad K_VALUE using feat flag#5988
feat: allow setting Kad K_VALUE using feat flag#5988mickvandijke wants to merge 2 commits intolibp2p:masterfrom
K_VALUE using feat flag#5988Conversation
af0abea to
4fdc5dc
Compare
|
Amended the commit to mutex all |
0a39dec to
78514ce
Compare
|
Updated the CI to run the |
Introduces new feature flags to change the `K_VALUE` for Kademlia. The new feat flags are: `k-4`, `k-8`, `k-12` and `k-16`.
78514ce to
0a1880e
Compare
0a1880e to
e9b7c94
Compare
|
Hi @mickvandijke, thank you for your PR! Citing @guillaumemichel from #5501 (comment):
Can you expand a little bit on why you need to modify 2) or 3)? Implementation wise @guillaumemichel is probably best to review this, but I don't think we should use feature-flags.
Two alternative solutions that came to my mind:
|
|
I agree with @elenaf9, feature flags are probably not the correct way to add configurability for
Both suggested solutions seem like a better way to proceed. |
Description
Introduces new feature flags to change the
K_VALUEfor Kademlia. The new feat flags are:k-4,k-8,k-12andk-16.Related issues:
Notes & open questions
I know that Config::set_kbucket_size is a thing, but that doesn't change the
K_VALUE, which influences many parts of the code.I have tried making the
K_VALUEconfigurable using a compile time environment variable and a build script, which would arguably be a cleaner solution, but I couldn't get it to work.So I thought using feat flags would be a nice, although maybe a bit unorthodox alternative, that doesn't require significant code refactoring.
Change checklist