-
Notifications
You must be signed in to change notification settings - Fork 23
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
feat(network): divide compression algorithm support in three categories #150
feat(network): divide compression algorithm support in three categories #150
Conversation
6b4872d
to
b05947d
Compare
ec329d1
to
f91660f
Compare
f91660f
to
575b557
Compare
…ies: supported, preferred and disabled BREAKING CHANGES: Compression in algorithm configuration format is changed as follows: ```toml algorithms = { lz4 = "preferred" } ```
575b557
to
bd294b5
Compare
elfo-network/src/config.rs
Outdated
#[serde(default)] | ||
pub algorithm: CompressionAlgorithm, | ||
pub algorithms: CompressionPreference, |
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.
This is a breaking change (which is ok because we have alpha versions), but, in this case, I think we can revise it even more and use system.network.compression.lz4 = "Preferred"
. If other settings (e.g. compression level) will be available to configure in the future, it will support the second per-codec form system.network.compression.zstd = { preference = "Preferred", level = 6 }
in a backward-compatible way. I don't see what common options can be added next to algorithms
to have this additional nesting level.
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.
Still "algorithms"
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.
gonna fix on my own
…modules, get rid of `CompressionAlgorithm` type
…ence`, add `Disabled` option in it and make algorithms config more backward compatible
de322c8
to
198be9d
Compare
198be9d
to
af6e7bb
Compare
Self(joined) | ||
} | ||
|
||
pub(crate) fn toggle(&mut self, algos: Algorithms, pref: Preference) { |
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.
Actually, It's not a "toggle", because this code does not work as expected for toggle(Preferred); toggle(Disabled)
sequence.
What does it mean? Both peers perform the same intersection process. |
This PR introduces more flexibility in compression algorithm selection by dividing "support" in three categories:
supported
- the algorithm can be used to communicate with the nodepreferred
- the algorithm should be selected to communicate with the nodedisabled
- the algorithm is not supportedTwo nodes decide on the algorithm as follows:
First of all, this algorithm implies there's node which "leads" in choice of algorithm ("first node"), in the current implementation, lead node is node that accepts the connection.
BREAKING CHANGES: compression algorithms in config expressed now as follows: