Wire per-topic partial-messages SubOpts flags (step 1)#460
Draft
lucassaldanha wants to merge 4 commits intolibp2p:developfrom
Draft
Wire per-topic partial-messages SubOpts flags (step 1)#460lucassaldanha wants to merge 4 commits intolibp2p:developfrom
lucassaldanha wants to merge 4 commits intolibp2p:developfrom
Conversation
Captures the MVP scope, jvm-libp2p/client responsibility boundary, client-facing API, routing semantics, per-group lifecycle and DoS caps, and the implementation plan for the gossipsub partial-messages extension. Lands ahead of implementation so sub-issues of libp2p#435 can reference a stable design anchor.
Step 1 of the partial-messages extension: plumb SubOpts.requestsPartial / SubOpts.supportsSendingPartial through subscribe announcements in both directions, and track the per-peer-per-topic receive state. - AbstractRouter parses the flags with the spec-mandated coercion (supportsSendingPartial := requestsPartial || supportsSendingPartial) and zeroes them on subscribe=false. - New enqueueSubscribe hook unifies outbound subscribe enqueueing so GossipRouter can attach per-topic flags in a single override. - GossipRouter exposes setTopicPartialFlags(topic, ...) to configure flags advertised for a locally-subscribed topic, and stores inbound flags in a new PartialSubscriptionState (plain HashMap on the pubsub event loop). State is cleaned on peer disconnect, topic unsubscribe, and per-peer unsubscribe. - Outbound unsubscribe MUST NOT carry partial flags; enforced at the SubscriptionPart wire-build site. No routing behaviour changes yet. See docs/partial-messages.md §4.5, §5, §6.1 for context.
…t addSubscription overload
- `PartialSubscriptionWireTest`: route reads of `partialSubscriptionState`
through `submitOnEventThread { ... }.join()`. The state container is
not thread-safe; direct access from the JUnit thread races the event
loop and can surface as `ConcurrentModificationException` or stale
reads. Two helpers (`peerFlagsOnEventLoop`, `snapshotPartialStateOnEventLoop`)
establish the happens-before barrier.
- `RpcPartsQueue`: remove the 2-arg `addSubscription(topic, status)`
default overload. The remaining 4-arg abstract method is the single
source of truth; `addSubscribe` / `addUnsubscribe` remain the
convenience entry points.
- `PartialSubFlags.coerce(requestsPartial, supportsSendingPartial)`: single source of truth for the spec coercion rule `supportsSendingPartial := requestsPartial || supportsSendingPartial`. Used from `GossipRouter.setTopicPartialFlags` for the outbound side. AbstractRouter keeps the inline expression for the receive side to avoid a reverse layering dependency (pubsub -> gossip); a comment notes the rule is applied on both sides. - `PartialSubscriptionState.setPeerFlags`: document that passing `PartialSubFlags.NONE` (or any equivalent all-false flags) is treated as a removal. Makes the set-sometimes-deletes invariant explicit for readers. - `AbstractRouter.handleMessageSubscriptions`: add Kdoc now that the method is `protected open`. Documents the "call super" contract for overrides (GossipRouter relies on this to keep peersTopics and partialSubscriptionState in sync) and the flag-normalisation precondition.
lucassaldanha
changed the title
lucassaldanha
changed the title
lucassaldanha
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Step 1 of the gossipsub partial-messages extension under #435.
Plumbs
SubOpts.requestsPartial/SubOpts.supportsSendingPartialthrough subscribe announcements in both directions, and tracks the per-peer-per-topic receive state. No routing behaviour changes yet — handler, RPC dispatch, publish, and routing rules are later steps.Addresses #444.
Scope
AbstractRouterparses the flags on inboundSubOptsand applies the spec coercionsupportsSendingPartial := requestsPartial || supportsSendingPartial. Flags are zeroed whensubscribe=false.AbstractRouter.enqueueSubscribe(queue, topic)hook unifies outbound subscribe enqueueing soGossipRoutercan attach per-topic flags in a single override (used by bothsubscribe()and theonPeerActivere-announce).GossipRouter:setTopicPartialFlags(topic, requestsPartial, supportsSendingPartial)— configures flags advertised for a locally-subscribed topic (router-level API; publicGossip/PubsubApisurface intentionally untouched until Step 2 / Expose application interface to use partial messages #446).PartialSubscriptionStatefield stores inbound flags per(topic, peer). PlainHashMap— single-threaded pubsub event loop invariant.SubscriptionPartwire-build site.Design reference:
docs/partial-messages.md§4.5, §5, §6.1.Out of scope (later steps)
PartialMessagesHandlerinterface (Expose application interface to use partial messages #446, Step 2).Rpc.PartialMessagesExtensiondispatch /publishPartial(Publish partial messages #445).onEmitGossip(Gossip routing integration for partial messages #456).GroupState, TTL, DoS caps (Refine DoS protection and other security/performance concerns #449).Test plan
libp2p/src/test/kotlin/io/libp2p/pubsub/gossip/PartialSubscriptionStateTest.kt— 10 unit tests for the new state container (set/remove/disconnect/topic-gc semantics).libp2p/src/test/kotlin/io/libp2p/pubsub/gossip/extensions/PartialSubscriptionWireTest.kt— 9 integration tests viaTwoRoutersTest:supportsSendingPartial-only; no flags when none configured; unsubscribe strips flags.supportsSendingPartial-only stored verbatim; both-false leaves state empty; unsubscribe ignores flags and clears prior state; disconnect clears state; local unsubscribe clears state for that topic only../gradlew :libp2p:test --tests "io.libp2p.pubsub.*"— all pubsub tests green (no regressions inGossipExtensionsMessageHandlingTest,GossipExtensionsStateTest, floodsub, etc.)../gradlew :libp2p:spotlessCheck— clean.Stacking note
This PR stacks on #457 (design-doc-only). Until that merges, the diff here will show the
docs/partial-messages.mdcommit as well; it will drop out after rebasing ondeveloppost-#457.