Skip to content

fix: 修复 config service 缓存不一致导致 agollo 偶尔不更新配置的问题 - #367

Merged
mergify[bot] merged 2 commits into
apolloconfig:masterfrom
WTIFS:fix/304
Oct 24, 2026
Merged

mergify[bot] merged 2 commits into
apolloconfig:masterfrom
WTIFS:fix/304

Conversation

@WTIFS

@WTIFS WTIFS commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

This pull request fixes #366

Summary by CodeRabbit

  • New Features

    • Notification synchronization now includes message details when available, helping preserve notification freshness across updates.
    • Message information is tracked independently for each namespace and retained across synchronization attempts.
  • Bug Fixes

    • Notification updates now retain observed message versions when retries follow a failed request.
    • Older notification responses without message details continue to work correctly.
    • Empty message information is omitted from synchronization requests.

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 60eadc99-16c2-4fb2-83d7-608e7b073017
📥 Commits

Reviewing files that changed from the base of the PR and between 2ced0e9 and aeaa42c.

📒 Files selected for processing: 4
  • component/remote/async.go
  • component/remote/notification_messages_test.go
  • env/config/config.go
  • env/config/notification_messages_test.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

The change stores notification message IDs per namespace and includes them in configuration sync request URLs. It also adds tests for message merging, response handling, retries, and compatibility with notifications that omit message details.

Changes: Notification message freshness

Layer / File(s) Summary
Message ID contract and storage
env/config/config.go, env/config/notification_messages_test.go
Notification now supports optional message details. The synchronized map retains the highest ID per namespace and key, and returns copied snapshots. Tests cover merging, isolation, and concurrent updates.
Message IDs in sync requests
component/remote/async.go, component/remote/notification_messages_test.go
Sync records notification message IDs before namespace synchronization. When IDs exist, GetSyncURI adds them as an escaped JSON messages query parameter. Tests cover response handling, retries, and notifications without message details.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: zouyx, zanderme

Merge Risk: ⚪ Minimal · up to aeaa4

The configuration request supplies the notification IDs needed to refresh a stale config-service cache. No issue requiring a fix before merge was established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main change: fixing inconsistent config-service caching that can cause agollo to miss configuration updates.
Linked Issues check Passed Issue #366 requires the client to prevent a stale config-service cache from hiding a published configuration. Sync records notification message IDs before the config request. GetSyncURI sends the …
Out of Scope Changes check Passed The source changes add notification-message storage, request serialization, and synchronization behavior for issue #366. The added tests verify this behavior and legacy compatibility. No unrelated cha…
Full details: Linked Issues check

Explanation

Issue #366 requires the client to prevent a stale config-service cache from hiding a published configuration. Sync records notification message IDs before the config request. GetSyncURI sends the recorded IDs in the escaped messages query parameter. The notification ID is updated only after a successful response. The new tests cover 200, 304, malformed responses, retries, legacy notifications, message merging, isolation, snapshots, and concurrent updates.

Full details: Out of Scope Changes check

Explanation

The source changes add notification-message storage, request serialization, and synchronization behavior for issue #366. The added tests verify this behavior and legacy compatibility. No unrelated change is demonstrated.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mergify

mergify Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

感谢您提出Pull Request,我会尽快Review。我会在1-2日内进行查看或者回复,如果遇到节假日可能会处理较慢,敬请谅解。

@nobodyiam nobodyiam left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已核对 #366 的修复实现及测试覆盖,当前 CI 全部通过,未发现阻塞性问题,同意合并。

@mergify
mergify Bot merged commit bcc4543 into apolloconfig:master Oct 24, 2026
7 checks passed
@mergify

mergify Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] config service 缓存不一致导致 agollo 偶尔不更新配置

2 participants