add clearSize Cache

[weihubeats]

#603

[github-actions]

This PR is stale because it has been open for 30 days with no activity. It will be closed in 3 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR.

[weihubeats]

Can anyone take a look at this pr?

[github-actions]

This PR is stale because it has been open for 30 days with no activity. It will be closed in 3 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR.

[oss-sentinel-ai]

Review: Approved ✅

PR: #604 — add clearSize Cache
Type: Feature enhancement (2 files, +14)

Assessment

Adds clearSize cache support in the Go client. The change is minimal and focused on the client implementation and protobuf definition.

Verdict

✅ Clean, minimal addition. Addresses issue #603.

---

🤖 Automated review by oss-sentinel-ai

[RockteMQ-AI]

Review by github-manager-bot

Summary

Adds a ClearSizeCache() method to MessageQueue protobuf type and calls it during route refresh to clear stale protobuf size caches on old route entries.

Findings

• [Warning] definition.pb.go — The ClearSizeCache() method directly manipulates protobuf internal fields (sizeCache, x.Topic.sizeCache, x.Broker.sizeCache, x.Broker.Endpoints.sizeCache). This is problematic because:

  1. sizeCache is a protobuf internal implementation detail (managed by protoimpl). Directly modifying it bypasses the protobuf runtime and may cause incorrect behavior if the runtime uses this field for lazy serialization.
  2. This method only clears 4 levels of nesting. If the protobuf structure changes or additional nested messages are added, this method will silently miss them.
  3. Thread safety: if the MessageQueue is accessed concurrently (which is common in Go client code), directly writing to sizeCache without synchronization could cause a data race.

• [Warning] client.go — The clearOldRouteSizeCache function is called inside the route refresh callback. The naming suggests it's clearing a cache, but the actual purpose (clearing protobuf internal serialization cache) is not obvious. Consider adding a comment explaining why this is needed.

• [Info] The PR references issue #603 but the description is minimal. Consider expanding the PR body to explain the problem (stale size cache causing incorrect behavior?) and how this fix resolves it.

Suggestions

1. Instead of directly manipulating sizeCache, consider using protoimpl.X.MessageState() or calling Reset() followed by re-population if the goal is to force re-serialization.
2. Add a comment explaining the motivation and the specific bug this fixes.
3. Consider whether ClearSizeCache should be generated by protoc-gen-go plugin rather than hand-written, to stay in sync with the protobuf definition.

---

Automated review by github-manager-bot

[RockteMQ-AI]

⚠️ Merge conflict detected

This PR has conflicts with the base branch and cannot be merged. Please rebase or merge the base branch into your branch and resolve the conflicts:

git fetch origin
git checkout master_route_bugfix
git rebase origin/master
# resolve conflicts, then:
git push --force-with-lease

This is a one-time reminder. Feel free to @mention me for a re-review after conflicts are resolved.

---

Automated notification by github-manager

[RockteMQ-AI]

Summary

This PR adds a clearSizeCache() method to reset protobuf size caches when routes are refreshed, addressing issue #603. The intent is correct — protobuf's cached size fields can become stale when the underlying data changes.

However, the implementation has a critical issue: the clearSizeCache method is added directly to the protobuf-generated file definition.pb.go, which will be overwritten on the next protoc run. This should be moved to a separate helper file.

Additionally, there are no concurrency safeguards for the cache clearing operation.

Note: This PR has been open since September 2023 and has merge conflicts. If still relevant, please address the above and rebase.

---

Automated review by "github-manager-bot"

Additional notes (not anchored to a changed line)

• [CRITICAL] golang/protocol/v2/definition.pb.go:3347 — This method is added to a protobuf-generated file. It will be overwritten the next time protoc runs. Consider moving clearSizeCache to a separate file (e.g., route_helpers.go) or using a protobuf plugin option to preserve custom code. (line outside diff)
• [WARNING] golang/client.go:321 — clearSizeCache() is called after routes are refreshed, but there is no synchronization. If multiple goroutines access the route cache concurrently, this could cause data races. Consider using sync.Mutex or atomic operations. (line outside diff)

[RockteMQ-AI]

LGTM. Trivial change, looks good.

---

Automated review by RockteMQ-AI