Skip to content

fix(portForwarding): serialize concurrent PF rule creation per VIP to prevent duplicates#3345

Open
zstack-robot-1 wants to merge 1 commit into5.5.6from
sync/ye.zou/fix/ZSTAC-77673
Open

fix(portForwarding): serialize concurrent PF rule creation per VIP to prevent duplicates#3345
zstack-robot-1 wants to merge 1 commit into5.5.6from
sync/ye.zou/fix/ZSTAC-77673

Conversation

@zstack-robot-1
Copy link
Collaborator

Problem\nConcurrent APICreatePortForwardingRuleMsg requests for the same VIP could create duplicate port forwarding rules with overlapping port ranges. The interceptor checks for port overlap before the handler, but without synchronization, two concurrent requests can both pass the check and persist duplicate rules.\n\nResolves: ZSTAC-77673\n\n## Root Cause\nThe handle(APICreatePortForwardingRuleMsg) method persists the PortForwardingRuleVO without any per-VIP synchronization. The existing PortForwardingApiInterceptor checks for VIP port overlap, but the window between the interceptor check and handler persist() allows race conditions.\n\n## Fix\n1. Wrap the CREATE handler in thdf.chainSubmit(new ChainTask) with sync signature portforwardingrule-vip-{vipUuid} — same pattern as the DELETE handler\n2. Re-check VIP port overlap inside the synchronized ChainTask before persist\n3. Add chain.next() at all async exit points (VIP acquire callbacks + FlowChain done/error)\n\nThis serializes all port forwarding rule creation per VIP, eliminating the race condition window.\n\n## Test Plan\n- Verify creating port forwarding rules normally still works\n- Verify concurrent creation of rules with overlapping ports on the same VIP returns error for the second request\n- Verify rules on different VIPs can still be created concurrently

sync from gitlab !9174

…tion per VIP to prevent duplicate rules

Resolves: ZSTAC-77673

Change-Id: I7f03df7bd22cd7d39097a34197313126bea811e1
@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

总览

通过使用 ChainTask 引入异步同步处理方式,替代了原有的直接方法序列。新增 VIP 端口范围重叠预检查,防止在创建端口转发规则前产生冲突。

变更

聚类 / 文件 总结
端口转发管理器异步重构
plugin/portForwarding/src/main/java/org/zstack/network/service/portforwarding/PortForwardingManagerImpl.java
引入 ChainTask 对 APICreatePortForwardingRuleMsg 进行异步同步处理;新增 VIP 端口范围重叠检查机制;重构流程链构建,添加显式的 chain.next() 调用;优化 VIP 获取时序,确保重叠检查在 VIP 获取之前执行;为 ChainTask 添加描述性名称覆盖。

序列图

sequenceDiagram
    actor Client
    participant PortForwardingManager
    participant ChainTask
    participant Database
    participant VIPService
    
    Client->>PortForwardingManager: APICreatePortForwardingRuleMsg
    PortForwardingManager->>ChainTask: 创建同步任务
    ChainTask->>Database: 查询同VIP、同协议的现有规则
    Database-->>ChainTask: 返回规则列表
    alt 存在端口范围重叠
        ChainTask->>ChainTask: 发布错误事件,停止流程
        ChainTask-->>Client: 返回错误
    else 无重叠
        ChainTask->>VIPService: 获取VIP详情
        VIPService-->>ChainTask: VIP信息
        ChainTask->>Database: 持久化端口转发规则
        Database-->>ChainTask: 成功确认
        ChainTask->>ChainTask: 发布成功事件
        ChainTask-->>Client: 返回规则创建结果
    end
Loading

预估代码审查工作量

🎯 4 (复杂) | ⏱️ ~45 分钟

诗歌

🐰 异步链式响当当,
VIP检查保安康,
端口重叠早预防,
同步处理如流畅,
转发规则新模样!


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 2 warnings)
Check name Status Explanation Resolution
Title check ❌ Error The PR title exceeds the 72-character limit at 88 characters, violating the specified formatting requirement. Shorten the title to 72 characters or less while maintaining the [scope]: format, e.g., 'fix(portForwarding): serialize PF rule creation per VIP to prevent duplicates'.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (2 files):

⚔️ network/src/main/java/org/zstack/network/service/DhcpExtension.java (content)
⚔️ plugin/portForwarding/src/main/java/org/zstack/network/service/portforwarding/PortForwardingManagerImpl.java (content)

These conflicts must be resolved before merging into 5.5.6.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the problem, root cause, fix approach, and test plan, all directly related to the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sync/ye.zou/fix/ZSTAC-77673
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch sync/ye.zou/fix/ZSTAC-77673
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@plugin/portForwarding/src/main/java/org/zstack/network/service/portforwarding/PortForwardingManagerImpl.java`:
- Around line 740-741: The FlowChain name in PortForwardingManagerImpl is
misspelled as "create-portforwading"; update the string passed to
flowChain.setName(...) (the FlowChain created via
FlowChainBuilder.newShareFlowChain() and stored in variable flowChain) to the
correct spelling "create-portforwarding" so the chain name is accurate.

Comment on lines +740 to +741
FlowChain flowChain = FlowChainBuilder.newShareFlowChain();
flowChain.setName("create-portforwading");
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

FlowChain 名称存在拼写错误。

"create-portforwading" 应为 "create-portforwarding"(缺少字母 'r')。

🐛 建议修复
 FlowChain flowChain = FlowChainBuilder.newShareFlowChain();
-flowChain.setName("create-portforwading");
+flowChain.setName("create-portforwarding");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
FlowChain flowChain = FlowChainBuilder.newShareFlowChain();
flowChain.setName("create-portforwading");
FlowChain flowChain = FlowChainBuilder.newShareFlowChain();
flowChain.setName("create-portforwarding");
🤖 Prompt for AI Agents
In
`@plugin/portForwarding/src/main/java/org/zstack/network/service/portforwarding/PortForwardingManagerImpl.java`
around lines 740 - 741, The FlowChain name in PortForwardingManagerImpl is
misspelled as "create-portforwading"; update the string passed to
flowChain.setName(...) (the FlowChain created via
FlowChainBuilder.newShareFlowChain() and stored in variable flowChain) to the
correct spelling "create-portforwarding" so the chain name is accurate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants