cubemaster: fix ceiling division in the scheduler concurrency limits

[dwin-gharibi]

Closes #1464.

Motivation

Three limit calculations were written as int64(math.Ceil(float64(a * b * 1.0 / c))). Every operand
is int64 and the untyped constant 1.0 is representable as an integer, so it converts to int64
and the whole inner expression is integer arithmetic — it truncates. math.Ceil then runs on an
already-integral value and does nothing.

The * 1.0 factors show the intent was float division and round-up. What actually happens is
round-down, so whenever the node counts do not divide evenly the cluster runs one unit below its
configured create/destroy concurrency.

What this changes

CubeMaster/pkg/scheduler/local.go — convert to float64 before dividing, at all three sites:

line	before	after
:283	float64(totalLimitCreate*1.0/newHealthyNodes)	float64(totalLimitCreate)/float64(newHealthyNodes)
:286	float64(newHealthyNodes * newCreateLimitOfEveryNode * 1.0 / newMasterNodes)	float64(newHealthyNodes*newCreateLimitOfEveryNode) / float64(newMasterNodes)
:309	float64(newHealthyNodes * newLimitDesroyOfEveryNode * 1.0 / newMasterNodes)	float64(newHealthyNodes*newLimitDesroyOfEveryNode) / float64(newMasterNodes)

The numerator product stays in int64 (it is a small count product, nowhere near overflow) and only
the division becomes floating point, which is the minimal change that makes math.Ceil meaningful.

No comment changes.

Testing

New: CubeMaster/pkg/scheduler/local_limit_math_test.go — table tests pinning the rounding for both
shapes, including the exact cases from the issue (10/3 → 4, 4*5/3 → 7) plus exact-division and
n < divisor cases so a future refactor cannot silently reintroduce truncation.

$ docker run --rm ... -w /w/CubeMaster golang:1.26 go test -short ./pkg/scheduler/...
ok  github.com/tencentcloud/CubeSandbox/CubeMaster/pkg/scheduler         0.004s
ok  github.com/tencentcloud/CubeSandbox/CubeMaster/pkg/scheduler/selctx  0.002s

CI gates checked locally:

• gofmt -l ./pkg — clean (fmt-check).
• GOOS=linux go build ./... — clean.
• staticcheck -checks 'SA4015' ./pkg/scheduler/ — the three local.go findings are gone.

Risk / rollout

Behaviour change by design: create and destroy concurrency will increase by one unit in clusters
where the division was not exact. That is the configured intent, but it does mean slightly more
in-flight work per node after this lands — worth noting if a cluster was unknowingly relying on the
lower effective limit.

pkg/selector/score/realtimescore_test.go:40-41 has the same math.Ceil-on-integer pattern in a
test; left alone here to keep this PR single-purpose.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[cubesandboxbot]

These tests assert on perNodeCreateLimit / clusterLimit — helpers that re-implement the fixed formulas rather than calling the production code. Since they're decoupled from the expressions inside monitorLimit() in local.go, a future change that reintroduces the * 1.0 integer-truncation pattern in local.go would leave these tests green, so the stated goal ("a future refactor cannot silently reintroduce truncation") isn't fully met. Consider extracting the per-node and cluster-limit computations into shared package-level functions that both monitorLimit() and these tests call, so the tests guard the real production path.

[cubesandboxbot]

Review: cubemaster: fix ceiling division in the scheduler concurrency limits (#1465)

Verdict: Approve — correct bug fix, one test-coverage improvement worth making

The PR correctly identifies and fixes a real bug. The three expressions of the form
int64(math.Ceil(float64(a * b * 1.0 / c))) were silently performing integer division:

• 1.0 is an untyped constant with value exactly 1, which is representable as int64, so in
  totalLimitCreate * 1.0 the constant is converted to int64 and the multiplication (and the
  subsequent / newHealthyNodes) is integer arithmetic, truncating toward zero.
• math.Ceil then runs on an already-integral float64 and is a no-op, so the effective result
  was floor (truncation), not the intended ceiling.

The fix (float64(...) conversions before the division) makes math.Ceil meaningful and matches
the evident intent. I verified:

• No divide-by-zero introduced. newHealthyNodes is guarded (<= 0 → return) before both
  sites, and localcache.HealthyMasterNodes() (localcache/export.go:487) clamps its result to
  >= 1, so newMasterNodes can never be 0.
• No overflow concern. The numerator products are node counts × concurrency limits — far below
  float64's exact-integer range (2^53), so the float division is exact for these magnitudes.
• Test math is correct. All 9 table cases check out (e.g. 10/3 → 4, 4*5/3 → 7,
  100/7 → 15, 100/3 → 34).
• The unchanged realtimescore_test.go:40-41 cases divide exactly (900/30, 900/3), so the
  truncation bug never manifested there — leaving that test alone is fine.
• Style is consistent with the repo (copyright header matches, gofmt-clean, max() builtin
  already in use at local.go:283, module is go 1.25.7 so no version concern).

The behavior change (limits round up by one unit in non-exact divisions) is intentional and
clearly documented in the PR description.

Finding (minor)

The new tests don't exercise the production code path. local_limit_math_test.go defines
package-level helpers perNodeCreateLimit / clusterLimit that re-implement the fixed formulas,
then asserts on those helpers. They are independent of the actual expressions inside
monitorLimit(), so a future refactor that reintroduces the * 1.0 integer-truncation pattern in
local.go would leave these tests green. The PR's stated goal — "a future refactor cannot
silently reintroduce truncation" — is therefore only partially met. Consider extracting the
per-node and cluster-limit computations into shared package-level functions used by both
monitorLimit() and the tests, so the tests guard the real production logic.

Optional suggestion

For these small positive-integer counts, an integer ceiling-division idiom — (a + b - 1) / b —
would compute the same result with no float conversion at all and no reliance on math.Ceil on
floats. The float approach is fine here (values are far below precision limits), so this is a
style preference, not a requirement.

---

AI-generated review — findings are advisory; a human maintainer should confirm before merging.