Fix dynamic PID notification fan-out#2517
Open
MrAlias wants to merge 4 commits into
Open
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2517 +/- ##
==========================================
- Coverage 69.24% 68.84% -0.41%
==========================================
Files 345 349 +4
Lines 46747 47456 +709
==========================================
+ Hits 32372 32672 +300
- Misses 12330 12723 +393
- Partials 2045 2061 +16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This comment was marked as resolved.
This comment was marked as resolved.
* Fix dynamic PID notification fan-out stalls * Preserve stalled PID notification subscribers Keep dynamic PID notifier fan-out from blocking on a full subscriber while still preserving that subscriber for future deliveries. A slow consumer now drains from its own mailbox instead of being removed from the notifier permanently. Update the full-subscriber tests to assert that active subscribers still receive updates and the temporarily full subscriber catches up once drained. Stub the ignored-path decorator test so it no longer depends on the host PID 1 executable. * Bound PID notifier subscriber backlog Bind internal dynamic PID notification subscriptions to their owning run context so shutdown removes subscribers from the notifier instead of leaving stale channels behind. Cap each subscriber mailbox backlog so legacy non-draining subscribers cannot retain every future PID. Keep the old channel API as a compatibility fallback while routing in-repo consumers through context-aware notification helpers. * Preserve live PID notifier backlogs Keep bounded pending queues for legacy dynamic PID notification subscribers, which have no cancellation path and can otherwise retain every future PID event. Allow context-bound subscribers to queue the full pending set until they drain or their context is canceled. The in-repo consumers use those subscriptions, so dynamic selector state no longer drops add or remove events during a temporary burst. * Preserve duplicate PID notify edges Keep every queued PID notification entry instead of deduplicating by PID inside the subscriber mailbox. Dynamic consumers treat add and remove notifications as transitions, so repeated PID entries must survive while a subscriber is backlogged. Legacy subscribers remain bounded by pending entry count, and context-bound subscribers keep the full queued sequence until they drain or cancel.
The repeated request-body recovery tests caused pkg/ebpf/common to fail at compile time, which also stopped the collectt lint analyzer before it could inspect the package. Keep one copy of each test so the package builds while preserving the coverage added for request-body recovery behavior.
The lint workflow now reaches golangci-lint after the duplicate test removal and reports the length assertion in the dynamic PID selector tests. Use testify's Len helper so the assertion matches the repository lint policy while keeping the same expected backlog bound.
Contributor
Author
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.
Motivation
DynamicPIDSelectorpreviously returned the same single-consumer channels fromAddedPIDsNotifyandRemovedNotify, making notification deliveries a work-queue instead of a broadcast and allowing competing consumers to steal add/remove batches.DynamicAppIPstrackers introduced additional subscribers which created races where the application matcher could miss remove notifications, leaving deselected PIDs instrumented and leaking telemetry.Description
addedNotifyandremovedNotifyto create a new buffered channel for each caller.AddedPIDsNotifyandRemovedNotifyto return per-subscriber channels via the new notifier methods.DynamicSignalProcessEventGateto subscribe once before its select loop so repeated process events do not register orphaned notification subscribers.Testing
go test ./pkg/appolly/discover -run TestDynamicPIDSelector