-
Notifications
You must be signed in to change notification settings - Fork 0
Closed
Labels
bugSomething isn't workingSomething isn't working
Description
KeyError in TrafficMatrixPlacement with mode: combine
Description
TrafficMatrixPlacement workflow step fails with KeyError when using traffic matrices with mode: combine.
Error
KeyError: '_src_region1/./rb|region2/./rb|zZOhg4UlSmW4a1rwVWKW1A'
Reproduction
Any scenario with a traffic matrix using mode: combine:
traffic_matrix_set:
example:
- source_path: region1/.*/rb
sink_path: region2/.*/rb
demand: 100000
mode: combine # <-- triggers the bug
Root Cause
In ngraph/exec/analysis/flow.py, both build_demand_context() and demand_placement_analysis() reconstruct TrafficDemand objects from serialized config but fail to pass the id field:
demand = TrafficDemand(
source_path=config["source_path"],
sink_path=config["sink_path"],
demand=config["demand"],
mode=config.get("mode", "pairwise"),
flow_policy_config=config.get("flow_policy_config"),
priority=config.get("priority", 0),
# id=config.get("id"), <-- MISSING
)
When mode: combine is used, pseudo-nodes are named using TrafficDemand.id (e.g., _src_{td.id}). Without preserving the ID, each reconstruction generates a new random UUID, causing pseudo-node name mismatches
between the cached context and the analysis phase.
Fix
Add id=config.get("id") to TrafficDemand construction in both:
- demand_placement_analysis() (line ~187)
- build_demand_context() (line ~433)
Testing Gap
This bug slipped through because:
1. No tests for mode: combine + context caching - The bug only occurs when both conditions are met
2. No TrafficMatrixPlacement integration test with mode: combine - Workflow tests likely only use mode: pairwise
3. No ID round-trip validation - No test verifies TrafficDemand.id survives serialization/deserialization
Suggested Test Coverage
| Test Type | Description |
|-------------|-------------------------------------------------------------------|
| Unit | TrafficDemand.id preserved through dict serialization round-trip |
| Integration | demand_placement_analysis() with cached context and mode: combine |
| E2E | TrafficMatrixPlacement step with mode: combine traffic matrix |Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working