2 issues found across 18 files

Confidence score: 3/5

• There is a concrete regression risk in src/go/plugin/framework/functions/scheduler.go: stopAccepting() flips accepting = false but only broadcasts when drained, while goroutines can block in enqueue() waiting for space; that combination can leave waiters stuck during shutdown/full-queue paths.
• The test concern in src/go/plugin/framework/functions/manager_flow_test.go is lower severity but meaningful: checking only an early "no 499" window can miss late 499s and allow false passes, reducing confidence that the behavior is fully covered.
• Given one medium-severity runtime-behavior issue (5/10, high confidence) plus a test-gap issue, this looks like some merge risk rather than a hard blocker.
• Pay close attention to src/go/plugin/framework/functions/scheduler.go and src/go/plugin/framework/functions/manager_flow_test.go - potential enqueue waiter wake-up gap and incomplete late-error validation.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/plugin/framework/functions/manager_flow_test.go">

<violation number="1" location="src/go/plugin/framework/functions/manager_flow_test.go:181">
P2: The test only verifies "no 499" in a short early window, so it can miss a late 499 and produce a false pass.</violation>
</file>

<file name="src/go/plugin/framework/functions/scheduler.go">

<violation number="1" location="src/go/plugin/framework/functions/scheduler.go:72">
P2: `stopAccepting()` sets `accepting = false` but only broadcasts when `drainedLocked()` (i.e. `pending == 0`). Since goroutines now block inside `enqueue()` waiting for space—and that only happens when the queue is full (`pending >= maxPending`)—`drainedLocked()` will be false and no broadcast fires. Blocked enqueuers won't see `!s.accepting` until another method happens to broadcast.

For consistency with the rest of this diff (which already made broadcasts unconditional in `next`, `cancelQueued`, `complete`), `stopAccepting()` should also broadcast unconditionally.</violation>
</file>

sequenceDiagram
    participant Netdata as Netdata Agent
    participant In as Stdin Reader
    participant FM as Functions Manager
    participant Sched as Keyed Scheduler
    participant Work as Framework Worker
    participant JM as JobMgr / SD
    participant SNMP as SNMP Collector
    participant Dev as SNMP Device

    Note over Netdata, Dev: Dyncfg Request Flow (with Back-pressure)

    Netdata->>In: Write FUNCTION command
    In->>FM: dispatchInvocation()
    
    FM->>Sched: NEW: enqueue(req)
    activate Sched
    Note right of Sched: CHANGED: Blocks if queue full (Size=1)
    Sched-->>FM: admitted
    deactivate Sched

    In-->>Netdata: Pipe read complete
    Note left of In: If Sched is full, Stdin read blocks,<br/>propagating back-pressure to Agent pipe.

    Sched->>Work: next()
    Work->>JM: NEW: enqueueDyncfgFunction(fn)
    Note right of JM: CHANGED: Blocking channel send.<br/>No handoff timeout.

    JM->>JM: dyncfgSeqExec()
    Note right of JM: CHANGED: Wait-decision gate is now<br/>indefinite (no 5s timeout).

    Note over Netdata, Dev: Cancellation Flow

    Netdata->>In: Write FUNCTION_CANCEL
    In->>FM: handleCancelEvent(uid)

    alt NEW: Request is still in Sched queue
        FM->>FM: Ignore cancel (allow side-effects to complete)
    else Request is currently running
        FM->>FM: Start fallback timer
        Note over FM: Delay: 5s
        FM->>FM: NEW: Silent markCancelled()
        Note right of FM: Tombstones UID and advances lane.<br/>Does NOT send 499 to Netdata.
        FM->>Sched: complete(key, uid)
    end

    Note over Netdata, Dev: SNMP Collection Flow (Periodic)

    SNMP->>Dev: CHANGED: SNMP GET (MaxOIDs=20)
    Note right of SNMP: Removed 32-bit Fallbacks.<br/>Requests 64-bit HC counters only.
    Dev-->>SNMP: PDU Response

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Pull request overview

Patch release 2.10.2 focusing on collector reliability and dyncfg/function pipeline correctness by reducing SNMP request size, removing ZFS crash conditions, and making dyncfg handoff non-droppable (blocking) after removing the wait-decision timeout.

Changes:

• SNMP: lower default MaxOIDs to 20 and make IF-MIB traffic/unicast metrics 64-bit HC only (no 32-bit fallback).
• Functions/dyncfg: remove wait-decision timeout and replace “queue full” rejections with blocking enqueue/back-pressure; adjust cancel semantics and runtime metrics/tests accordingly.
• Diskspace (ZFS): guard NULL filesystem, cache pool capacity using existing statvfs() results, and move LXC detection to a one-time startup check.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.