Adjust resourcetopo directref relation update logic by AnnaYue · Pull Request #106 · KusionStack/kube-utils

AnnaYue · 2026-07-24T09:57:53Z

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

N
Y

2. What is the scope of this PR (e.g. component or file name):

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

N
Y

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

6. Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

need to split to atomic reference pair to do sort and compare

This commit addresses potential deadlock issues and adds timeout protection for event handlers. Lock Hierarchy Fixes: - Add doc.go with comprehensive lock hierarchy documentation (7 levels) - Fix checkGC(): acquire lock before metaLock, release between - Fix checkLabelUpdateForPostNode(): copy data under lock, release before processing - Fix updateNodeMeta(): release metaLock before acquiring lock - Fix deleteAllRelation(): restructure to follow preNode->postNode lock order - Add storagesLock RWMutex to protect storages map from concurrent access Event Handler Timeout: - Add EventHandlerTimeout config option (default 1s) - Implement timeout in handleNodeEvent() with context cancellation - Implement timeout in handleRelationEvent() with context cancellation - Handlers run sequentially; timeout skips remaining handlers after warning Other Changes: - Change event queue log level from Infof to V(6).Infof - Add threading model documentation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Change map get operations to use the two-value lookup pattern (val, ok := map[key]) to explicitly check for key existence. This prevents returning nil values that could cause NPE when callers try to use them. Changes: - manager.go: AddNodeHandler, GetTopoNodeStorage, GetNode now check ok before using value - node_storage.go: getNode and deleteNode check ok for cluster, namespace, and name lookups - GetClusterNode: return nil early if node not found Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Extract duplicate timeout logic into two helper methods: - executeHandlers(): for NodeHandler with timeout support - executeRelationHandlers(): for RelationHandler with timeout support This reduces code duplication and makes the main event handling logic cleaner. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…, and timeout metrics - Add context.Context to NodeHandler and RelationHandler for cooperative cancellation. When EventHandlerTimeout expires, ctx is cancelled so handlers can select on ctx.Done() and return cleanly (no goroutine leak). - Add defer/recover() to executeHandlers and executeRelationHandlers to catch handler panics and prevent process crashes. Panics are logged and the done channel is always closed. - Add HandlerTimeoutMetrics() returning atomic counters for timed-out node and relation handler callbacks. - Fix checkGC() to use metaLock.RLock() instead of Lock() for a read-only boolean check, matching isObjectExisted(). - Revert GetClusterNode nil-return behavior change to preserve the original API contract for deleted-but-referenced nodes. - Update lock hierarchy docs: swap Level 4 (relationsLock) and Level 5 (lock) to match actual code ordering. - Update doc.go callback threading docs to reflect goroutine + timeout execution model. - Add tests: cooperative handler no-leak, panic recovery, goroutine leak detection, DirectRef delta updates, lock ordering stress. Co-Authored-By: Claude <noreply@anthropic.com>

…ct{} for map set - Replace executeHandlers/executeRelationHandlers (40-line copy-paste) with a single generic executeHandlers[H any]() function. - Change map[nodeMeta]interface{} to map[nodeMeta]struct{} for idiomatic Go set semantics (zero-value sentinel, zero allocation). Co-Authored-By: Claude <noreply@anthropic.com>

…es, queue depth - Add execMetrics struct with timeouts/panics/active counters passed to the generic executeHandlers function. - Add Metrics() to Manager interface returning Metrics struct with: NodeHandlerTimeouts, RelationHandlerTimeouts, NodeHandlerPanics, RelationHandlerPanics, ActiveHandlerGoroutines, NodeEventQueueDepth, RelationEventQueueDepth. - Counters are atomic.Int64, updated inside executeHandlers. - Tests verify timeout, panic, and active goroutine metrics. Co-Authored-By: Claude <noreply@anthropic.com>

… shadow builtin 'min', and gofmt formatting Co-Authored-By: Claude <noreply@anthropic.com>

zoumo

审查发现 2 个 P1、1 个 P2 问题，建议修复后合并。

独立整体风险：高。变更涉及通用资源拓扑缓存和所有 node/relation callback 的执行路径；并发更新可能造成持续错误的关系或节点丢失，共享超时预算可能导致正常多订阅者配置丢失通知，影响依赖这些通知的控制器协调。没有证据表明会直接破坏 Kubernetes API 中的资源。

审查范围：c81662b5b6a373aa7ca546a5b63a43f6ebb0f195 → 61cdbdc，完整核对 11 个文件、+1069/-113。由独立审查子智能体完成，结论基于源码控制流及并发交错分析，未运行测试。

zoumo · 2026-10-10T07:31:36Z

-	for _, relation := range node.labelRelations {
+	labelRelationsCopy := make([]ResourceRelation, len(node.labelRelations))
+	copy(labelRelationsCopy, node.labelRelations)
+	node.relationsLock.RUnlock()


[P1] 保持 selector 检查与关系修改的一致性

checkLabelUpdateForPostNode 复制 labelRelations 后立即释放 relationsLock，随后按旧快照增删真实关系。后置资源 informer 在此暂停时，前置资源 informer 可以通过 OnUpdate 将 selector 从 app=a 改为 app=b，删除原关系并完成更新；前一个 goroutine 恢复后仍会按旧 selector 调用 rangeAndSetLabelRelation，重新加入已不应存在的边。反方向也能删除新 selector 正确建立的边。此前读锁覆盖整个检查与修改过程，阻止该交错。需要保留原子性或增加版本检查/重试，否则图结构与事件通知可持续偏离最新配置。

zoumo · 2026-10-10T07:31:36Z

+	// Then acquire metaLock to check object existence (Level 6)
+	n.metaLock.RLock()
+	objectExisted := n.objectExisted
+	n.metaLock.RUnlock()


[P1] 将 GC 条件检查与删除保持为一致操作

新实现分别释放关系锁和 metadata 锁后，仍依赖旧的 relationsEmpty、objectExisted 快照直接按名称删除。一个 informer 删除最后一条 DirectRef 并进入 checkGC 时，目标资源 informer 可以在 metaLock.RUnlock() 后完成该节点的 OnAdd，或另一类前置资源完成新的 DirectRef 建立；随后 GC 仍将节点从 storage 移除。结果是已存在节点无法再通过 GetNode 查到，或新关系持有已脱离 storage 的节点，后续同名资源创建又产生不同节点实例。需要协调 storage 删除与节点状态复核，而非使用解锁后的快照。

zoumo · 2026-10-10T07:31:36Z

 			continue
 		}

+		ctx, cancel := context.WithTimeout(context.Background(), m.eventHandlerTimeout)


[P2] 为每个 handler 单独创建超时预算

EventHandlerTimeout 声明为每个 callback 的时限，但这里在遍历前只创建一个 context，executeHandlers 顺序执行全部 handlers 并共享它。默认 1 秒时，两个分别需要 600ms 的正常 handler 会导致第二个仅运行约 400ms 就被取消，第三个及后续 handler 被跳过，事件随后 Done 且没有补发；即使没有任何单个 handler 超过配置时限也会丢通知。关系事件有相同问题。应在每次 callback 开始时创建独立 timeout，并将该 context 传给对应 callback。

AnnaYue and others added 7 commits July 24, 2026 18:19

adjust directref relation update logic

135782e

need to split to atomic reference pair to do sort and compare

rm unused func

67bb35a

AnnaYue force-pushed the adjust-resourcetopo-directref-relation-update-logic branch from 13dcc94 to 078044d Compare July 25, 2026 11:27

AnnaYue and others added 2 commits July 25, 2026 23:38

fix(resourcetopo): fix lint issues — misspell 'cancelled'→'canceled',…

61cdbdc

… shadow builtin 'min', and gofmt formatting Co-Authored-By: Claude <noreply@anthropic.com>

AnnaYue requested review from ColdsteelRail and zoumo July 31, 2026 07:42

zoumo reviewed Oct 10, 2026

View reviewed changes

Navigation Menu

Repository navigation

Adjust resourcetopo directref relation update logic - #106

Adjust resourcetopo directref relation update logic#106
AnnaYue wants to merge 9 commits into
mainfrom
adjust-resourcetopo-directref-relation-update-logic

AnnaYue commented Jul 24, 2026

Uh oh!

zoumo left a comment

Uh oh!

zoumo Oct 10, 2026

Uh oh!

zoumo Oct 10, 2026

Uh oh!

zoumo Oct 10, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

AnnaYue commented Jul 24, 2026

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

2. What is the scope of this PR (e.g. component or file name):

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

6. Release note

Uh oh!

zoumo left a comment

Choose a reason for hiding this comment

Uh oh!

zoumo Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

zoumo Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

zoumo Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants