thc1006 commented on PR #1053: URL: https://github.com/apache/yunikorn-k8shim/pull/1053#issuecomment-5032857565
Thanks, this reads a lot cleaner. I've pushed the rework: - Validation now runs inside the existing task-group loop, no second pass. - `validateTaskGroupResources(taskGroup, totals)` takes the shared `totals` and updates it directly. - Per resource: sign check first, then the cpu->vcore canonical, then your `maxVal := math.MaxInt64/(members*milliConvert)` with `CmpInt64`. Dropped `checkedResourceValue` and `maxMilliCPU`. One place I didn't fold in: I kept a small per-group `seen` set that rejects a same-canonical-key collision (cpu together with vcore, or an explicit `pods`). GetTGResource adds the implicit `pods` first and then iterates the map, so an explicit `pods` deterministically overwrites that count, and cpu vs vcore overwrite each other by map order. Rejecting felt safer than passing a silently-overwritten ask to the core, but I'm happy to drop it and let them sum if you'd rather. Smaller note on the cpu bound: `maxVal` floor-divides by 1000, so it conservatively rejects cpu in the sub-milli remainder above roughly 9e15 cores (`MilliValue` rounds up). That's fail-safe and well past anything real, so I left your simpler bound, but I can make it an exact Quantity limit if you want it precise. I also strengthened the property test to check each accepted ask against an exact big.Int recomputation instead of just the sign, which pins the validator's canonicalization to GetTGResource's. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
