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

feat(iroh-gossip): configure the max message size #2340

Merged
merged 7 commits into from
Jun 5, 2024

Conversation

ramfox
Copy link
Contributor

@ramfox ramfox commented Jun 3, 2024

Description

Add configuration option for max_message_size for iroh-gossip::proto::Config.

This Config gets used in iroh-gossip::Gossip::from_endpoint.

iroh-docs still uses the default 4096 bytes. The max_message_size configuration is useful for folks using iroh-gossip::Gossip as its own library.

closes #2312

Breaking Changes

Adds:
iroh-gossip::Gossip::max_message_size - that reports the configured maximum message size for the gossip actor.

Changes:
iroh_gossip::net::util::read_message now takes a max_message_size: usize parameter
iroh_gossip::net::util::write_message now takes a max_message_size: usize parameter
iroh_gossip::net::util::read_lp now takes a max_message_size: usize parameter

Removes:
iroh-gossip::proto:: MAX_MESSAGE_SIZE const

Change checklist

  • Self-review.
  • Documentation updates if relevant.
  • All breaking changes documented.

@ramfox ramfox self-assigned this Jun 3, 2024
@ramfox ramfox added this to the v0.18.0 milestone Jun 3, 2024
@ramfox ramfox marked this pull request as ready for review June 3, 2024 19:08
@ramfox ramfox requested review from Frando and divagant-martian June 3, 2024 19:08
Copy link
Contributor

@divagant-martian divagant-martian left a comment

Choose a reason for hiding this comment

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

LGTM, maybe add some comments about a couple of caveats.

For example:

  • This should be a network-wide configuration parameter to ensure all nodes can transmit and read large messages.
  • This should be large enough to allow for gossip control messages to be sent.
  • Any other we can think of

iroh-gossip/src/proto/state.rs Outdated Show resolved Hide resolved
iroh-gossip/src/proto/topic.rs Outdated Show resolved Hide resolved
iroh-gossip/src/proto/topic.rs Show resolved Hide resolved
@ramfox ramfox force-pushed the feat/configure_max_message_size branch from ca4fc44 to 132dfd0 Compare June 4, 2024 23:28
@ramfox ramfox force-pushed the feat/configure_max_message_size branch from 127ae15 to da59b54 Compare June 5, 2024 17:29
@ramfox ramfox force-pushed the feat/configure_max_message_size branch from da59b54 to 4f83ab7 Compare June 5, 2024 17:56
@ramfox ramfox added this pull request to the merge queue Jun 5, 2024
Merged via the queue into main with commit 7153a38 Jun 5, 2024
24 of 25 checks passed
@ramfox ramfox deleted the feat/configure_max_message_size branch June 5, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

feat: make iroh-gossip message size configurable
3 participants