fix: Fix scale rule cache eviction by using rule ids(#6972)

[juicewcode]

Fixes #6972
Fixes #6623
Fix scale rule cache consistency for create, update, and delete operations.

ScaleRuleCache stores rules in a map keyed by metricName, while database operations identify rules by their primary key id. This difference could cause cached rules to become inconsistent with the database:

• A newly created rule could be cached with a different id from the database
  record.
• Renaming a rule could leave the old metric-name entry in the cache.
• Deleting a rule by id could fail to remove its cache entry because the id was
  incorrectly treated as a metric-name key.

##Changes

• Cache the same ScaleRuleDO instance that is inserted into the database,
  preserving the database-generated id.
• Remove the old metric-name cache entry when a rule is renamed.
• Add removeRulesByIdsFromCache to find cached rules by their database ids and remove them using their metric-name map keys.
• Update ScaleRuleServiceImpl#delete to use removeRulesByIdsFromCache.

These changes ensure that the in-memory scale-rule cache remains consistent with the database after rules are created, renamed, or deleted.

Make sure that:

• You have read the contribution guidelines.
• You submit test cases (unit or integration tests) that back your changes.
• Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

[Aias00]

This delete-by-ID eviction still depends on the cached rule ID matching the database row ID, but this PR is based on master, where ScaleRuleServiceImpl#create still inserts one ScaleRuleDO and then caches a second ScaleRuleDO built from the blank-id DTO. ScaleRuleDO.buildScaleRuleDO generates a fresh UUID when the DTO id is empty, so rules created through the service can still have a cached id that differs from the DB id being deleted. Could you include the create-side cache fix here, or base this after the fix from #6973, so delete-by-ID eviction can actually find newly created cached rules?

[juicewcode]

This delete-by-ID eviction still depends on the cached rule ID matching the database row ID, but this PR is based on master, where ScaleRuleServiceImpl#create still inserts one ScaleRuleDO and then caches a second ScaleRuleDO built from the blank-id DTO. ScaleRuleDO.buildScaleRuleDO generates a fresh UUID when the DTO id is empty, so rules created through the service can still have a cached id that differs from the DB id being deleted. Could you include the create-side cache fix here, or base this after the fix from #6973, so delete-by-ID eviction can actually find newly created cached rules?

This change is based on the cache consistency fix from PR #6973 .

The following cache consistency issues are addressed:

• Newly created rules are cached using the same ScaleRuleDO instance that was inserted into the database, ensuring that
  the cached rule id matches the database id.
• For deletion, cached rules are located by their database primary keys and removed using their metric-name cache keys.
• When a rule's metricName is changed, the previous metric-name cache entry is removed before the updated rule is cached.

The PR body has also been updated to reflect these changes and the problems they resolve.

[Aias00]

Review: fix: Fix scale rule cache eviction by using rule ids (#6972)

Reviewed at head 773b5b5. This addresses my earlier COMMENTED concern about delete-by-id eviction depending on a cached id that didn't match the DB row id. The fixes are correct:

1. Create-side cache consistency (lines 52-53)
create now caches the same scaleRuleDO instance that was inserted (addOrUpdateRuleToCache(scaleRuleDO)) instead of rebuilding a second ScaleRuleDO via buildScaleRuleDO(scaleRuleDTO). Since scaleRuleDO carries the id that insertSelective persists, the cached rule.getId() now matches the DB primary key — so delete-by-id eviction can actually locate the cached entry. This was the core of my prior comment and is resolved.

2. removeRulesByIdsFromCache (lines 24-31)
New method removes cached rules whose rule.getId() is in the supplied id set. Correct for a ConcurrentHashMap<String, ScaleRuleDO> keyed by metricName: it scans and removes by primary key rather than by metricName. ruleCache.remove(metricName, rule) is the atomic conditional-remove, safe under concurrent use.

3. Update stale metricName key (lines 61-69)
update now loads before, and when before.metricName != after.metricName it removes the old metricName key before re-adding — closing the stale-key duplication I flagged on #6973.

Non-blocking coordination note (important): this PR and #6974 both modify ScaleRuleServiceImpl.update. Here update uses updateByPrimaryKey(after) (full update) and caches after; #6974 uses updateByPrimaryKeySelective(after) + caches a re-selected persisted row. These two update implementations conflict and cannot both merge cleanly. Please consolidate into one canonical update: prefer updateByPrimaryKeySelective + cache the re-selected persisted row + keep the old-metricName-key removal from this PR. Either rebase one onto the other or fold the changes together before merge.

Functionally correct; approving. Please resolve the overlap with #6974 during merge.

[juicewcode]

Rebased onto latest master and resolved the conflict in ScaleRuleServiceImpl.update. The method now uses updateByPrimaryKeySelective + re-select persisted row for caching (from #6974), with the old-metricName-key eviction logic folded in before the cache refresh (from this PR). All other changes (create fix, removeRulesByIdsFromCache, delete update) remain intact. Ready for re-review.

[Copilot]

🟡 Changes recommended

Rename eviction can remove another rule sharing the old metric name.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes scale-rule cache consistency across create, rename, and delete operations.

Changes:

• Reuses persisted rule IDs when caching.
• Adds ID-based cache eviction.
• Removes stale metric-name entries during renames.

File summaries

File	Description
ScaleRuleServiceImpl.java	Updates cache synchronization for CRUD operations.
ScaleRuleCache.java	Adds ID-aware rule eviction.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Aias00]

Approved as PMC (Aias00). Coherent fix with regression tests; green CI, mergeable. Reviewed the diff.

[Aias00]

Good catch on a key-type mismatch.

ScaleRuleCache is keyed by metric name, but ScaleRuleServiceImpl.delete(ids) passes primary keys (ScaleRuleDO#getId). removeRulesFromCache(ids) therefore called ruleCache.remove(id) against a metric-name keyspace and removed nothing — so deleted scale rules stayed in the cache and kept being evaluated after their DB row was gone. This is exactly the kind of bug that only shows up as "the autoscaler keeps scaling for a rule I deleted".

The new removeRulesByIdsFromCache matches on rule.getId() instead, which is correct.

Implementation notes:

• Copying ids into a HashSet keeps the per-entry lookup O(1) instead of O(n) per rule.
• ruleCache.forEach((metricName, rule) -> ruleCache.remove(metricName, rule)) uses the two-arg ConcurrentHashMap.remove(k, v), which is an atomic conditional remove — safe to call while iterating a ConcurrentHashMap, unlike Map#remove(k) on a plain map during iteration.
• The runAfterCommit wrapper is preserved, so the cache is only purged once the delete actually commits. Good — purging before commit would leave a window where a rollback leaves the cache empty.

One ask (non-blocking but worth doing): there's no test for removeRulesByIdsFromCache. Given the whole point of this fix is that the old call silently no-op'd, a small unit test that seeds the cache with a rule and asserts it's gone after removeRulesByIdsFromCache(List.of(rule.getId())) would lock the behaviour in — and would have caught the original bug.