Skip to content

Feat: Add ZK config cache - #3612

Open
ywxzm03 wants to merge 19 commits into
apache:developfrom
ywxzm03:feat/zk-config-cache
Open

Feat: Add ZK config cache#3612
ywxzm03 wants to merge 19 commits into
apache:developfrom
ywxzm03:feat/zk-config-cache

Conversation

@ywxzm03

@ywxzm03 ywxzm03 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What this PR does:

This PR adds a read-through cache for the ZooKeeper Config Center to reduce repeated ZooKeeper reads from GetProperties and GetInternalProperty. The existing CacheListener updates or invalidates the cache when configuration change events are received.

Main changes:

  1. Add a configuration cache. Cache keys use the full ZooKeeper path, including the group and namespace.
  2. Add the config-center.cache-ttl configuration:
    • The default value is 30s;
    • Setting it to 0 disables the cache;
    • Negative values and invalid duration formats are rejected.
  3. Distinguish between:
    • An existing node with empty content;
    • A node that does not exist.
  4. Read from ZooKeeper on a cache miss, selecting the read method based on the watch state:
    • Use ordinary Get when the watch is active;
    • Use GetW to read the data and register a data watch when the watch is inactive;
    • Use ExistsW to watch for node creation when the node does not exist.
  5. Let CacheListener update the cache when add, update, and delete events are received:
    • Add/update events write the latest content to the cache and refresh the TTL;
    • Delete events cache the node as absent and refresh the TTL.
  6. Add an active state to track whether the watch for a specific configuration path is still valid, preventing duplicate watch registration during TTL refreshes.
  7. Clear the configuration cache and reset all watch states to inactive after a ZooKeeper reconnect. A generation counter prevents reads already in progress and old watch events from repopulating the cache after reconnection.
  8. Fix group/option path handling in AddListener and `RemoveListener so that the read path, listener path, and listener removal path remain consistent.
  9. Preserve the existing Base64 configuration behavior. Base64 decoding is still performed when a value is read from the cache.

This PR also adds tests covering cache behavior, TTL, watch-based updates, deletion, reconnect handling, generation isolation, and Base64 reads.

Design options:

This PR evaluated the following three approaches:

  1. TTL-only caching

    Advantages:

    • Simple to implement;
    • Does not require maintaining watch state for individual configuration paths;
    • TTL limits the maximum cache lifetime.

    Disadvantages:

    • Configuration changes are not reflected in the cache immediately;
    • Old values may be returned until the TTL expires;
    • ZooKeeper still needs to be accessed periodically;
    • It cannot make use of the existing CacheListener change notifications.
  2. Watch-driven caching only

    Advantages:

    • Cache updates can be applied promptly after configuration changes;
    • Under normal conditions, repeated ZooKeeper reads can be minimized.

    Disadvantages:

    • ZooKeeper watches are one-shot and must be continuously re-registered;
    • Watch registration failures, event content read-back failures, and reconnect scenarios are more complex;
    • If an event is missed or the watch state becomes unreliable, stale values may remain in the cache for an indefinite period;
    • Node absence, node creation, deletion, and rapid recreation must all be handled.
  3. TTL plus watch-driven caching

    Advantages:

    • Normal configuration changes update the cache promptly through watch events;
    • TTL provides a fallback when watch registration fails, event content read-back fails, or the client reconnects;
    • Ordinary Get is used when the watch is active, avoiding duplicate watch registration;
    • GetW/ExistsW are used to re-establish the watch when it is inactive;
    • It is more responsive than the TTL-only approach and more resilient than the watch-only approach.

    Disadvantages:

    • The implementation needs to maintain both cache TTLs and per-path watch state;
    • Watch consumption, reconnects, and concurrent reads require additional lifecycle handling;
    • The implementation is more complex than the TTL-only approach.

The TTL plus watch-driven approach was selected because it satisfies all of the following goals:

  • Reduce repeated ZooKeeper reads;
  • Reflect configuration changes in the cache promptly;
  • Fall back to ZooKeeper through TTL when the watch becomes invalid or fails;
  • Reduce the risk of stale cache data after reconnects or event-processing failures.

active state design:

ZooKeeper watches are one-shot. After a configuration change is received, the existing watch is consumed. The cache therefore needs to know whether a valid watch still exists for each concrete configuration path.

  • active=true: The path has a valid watch. When the TTL expires, ordinary Get is used to refresh the content without registering another watch.
  • active=false: The path does not have a valid watch. When the TTL expires, GetW is used to read the content and register a new data watch. If the node does not exist, ExistsW is used to watch for its creation.

This state prevents duplicate watch registration during TTL refreshes and ensures that watches are re-established after they are consumed.

Reconnect handling:

After a ZooKeeper reconnect, the cache content and local watch state established before the reconnect can no longer be treated as reliable. The reconnect callback therefore:

  1. Clears all configuration cache entries;
  2. Resets all concrete-path watch states to inactive;
  3. Increments the generation counter;
  4. Discards reads that started before the reconnect and old watch events, preventing old data from being written back into the cache after the reconnect.

The next configuration read after reconnecting accesses ZooKeeper again and re-establishes the watch based on the current node state.

Which issue(s) this PR fixes:

Fixes #3572

Does this PR introduce a user-facing change?:

None

This PR adds the following user-configurable option:

config-center.cache-ttl

@codecov-commenter

codecov-commenter commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 52.81385% with 218 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.88%. Comparing base (60d1c2a) to head (6a309a2).
⚠️ Report is 934 commits behind head on develop.

Files with missing lines Patch % Lines
config_center/zookeeper/impl.go 6.94% 62 Missing and 5 partials ⚠️
config_center/zookeeper/listener.go 42.59% 52 Missing and 10 partials ⚠️
config_center/zookeeper/config_cache.go 75.98% 44 Missing and 17 partials ⚠️
remoting/zookeeper/listener.go 0.00% 28 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3612      +/-   ##
===========================================
+ Coverage    46.76%   54.88%   +8.11%     
===========================================
  Files          295      476     +181     
  Lines        17172    37223   +20051     
===========================================
+ Hits          8031    20428   +12397     
- Misses        8287    15145    +6858     
- Partials       854     1650     +796     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread config_center/zookeeper/config_cache.go Outdated
Comment thread config_center/zookeeper/config_cache.go
Comment thread config_center/zookeeper/impl.go
Comment thread config_center/zookeeper/listener_test.go
@ywxzm03
ywxzm03 force-pushed the feat/zk-config-cache branch from 1b8232c to 2f7821f Compare August 14, 2026 19:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a read-through, TTL-based cache to the ZooKeeper Config Center with watch-driven updates, aiming to reduce repeated ZooKeeper reads while keeping cached configuration fresh via existing change events and safer reconnect handling.

Changes:

  • Introduces an LRU + TTL config cache with per-path watch state, auto-watch limiting, and generation-based reconnect isolation.
  • Updates ZooKeeper config-center listener/watch handling to keep cache entries in sync on add/update/delete events and to avoid duplicate watch registration.
  • Adds configuration parsing for config-center.cache-ttl plus extensive unit tests for cache/watches/reconnect/base64 behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
remoting/zookeeper/listener.go Adds optional watch-state callbacks and conditional watch re-registration behavior for configuration events.
config_center/zookeeper/listener.go Integrates cache/watch-state tracking into CacheListener, plus reconnect-time watch restoration logic.
config_center/zookeeper/listener_test.go Expands listener tests to cover cache interactions and watch state transitions.
config_center/zookeeper/impl.go Wires in the new cache, adds TTL parsing, updates GetProperties to use read-through cache + watch logic, and resets on reconnect.
config_center/zookeeper/impl_test.go Adds tests for watch selection, watch limits, watch-updated cache reads, base64 decoding from cache, and reconnect/reset behavior.
config_center/zookeeper/config_cache.go Implements the LRU+TTL cache, auto-watch limiting, per-path locking, and generation-based reset behavior.
config_center/zookeeper/config_cache_test.go Adds concurrency, eviction, expiry, reset, and watch-bounding tests for the cache implementation.
common/constant/key.go Adds the config-center.cache-ttl constant.

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

Comment thread config_center/zookeeper/config_cache.go
Comment thread config_center/zookeeper/listener.go
@Alanxtl

Alanxtl commented Aug 17, 2026

Copy link
Copy Markdown
Member

we are fixing the ci fail #3669

Comment thread config_center/zookeeper/impl_test.go
@sonarqubecloud

Copy link
Copy Markdown

@ywxzm03

ywxzm03 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

执行make test-race时,TestCacheListenerRemoveListenerDropsWatchAtAutoLimitTestRestartCallBackResetsCache发生了数据竞争;原因是单测中执行到defer cluster.Stop()时,停止 ZooKeeper 服务端,接着Conn.loop检测到网络断开,触发的setStateremoveWatcher()中的invalidateWatcher()存在并发访问窗口,发生了state的读写竞争(前者是原子写,但后者却是普通读);由于后来reviewer提出要引入zk服务端依赖,这个数据竞争问题得以才暴露出来;

若要解决这个问题,可以在dubbogo/go-zookeeper中将invalidateWatcher的state读取改为已有的原子读取State()即可,是否需要我去dubbogo/go-zookeeper那提个pr改一下这个小问题?

@Alanxtl

Alanxtl commented Aug 20, 2026

Copy link
Copy Markdown
Member

执行make test-race时,TestCacheListenerRemoveListenerDropsWatchAtAutoLimitTestRestartCallBackResetsCache发生了数据竞争;原因是单测中执行到defer cluster.Stop()时,停止 ZooKeeper 服务端,接着Conn.loop检测到网络断开,触发的setStateremoveWatcher()中的invalidateWatcher()存在并发访问窗口,发生了state的读写竞争(前者是原子写,但后者却是普通读);由于后来reviewer提出要引入zk服务端依赖,这个数据竞争问题得以才暴露出来;

若要解决这个问题,可以在dubbogo/go-zookeeper中将invalidateWatcher的state读取改为已有的原子读取State()即可,是否需要我去dubbogo/go-zookeeper那提个pr改一下这个小问题?

提一下吧

Comment thread config_center/zookeeper/impl.go
@ywxzm03

ywxzm03 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

执行make test-race时,TestCacheListenerRemoveListenerDropsWatchAtAutoLimitTestRestartCallBackResetsCache发生了数据竞争;原因是单测中执行到defer cluster.Stop()时,停止 ZooKeeper 服务端,接着Conn.loop检测到网络断开,触发的setStateremoveWatcher()中的invalidateWatcher()存在并发访问窗口,发生了state的读写竞争(前者是原子写,但后者却是普通读);由于后来reviewer提出要引入zk服务端依赖,这个数据竞争问题得以才暴露出来;
若要解决这个问题,可以在dubbogo/go-zookeeper中将invalidateWatcher的state读取改为已有的原子读取State()即可,是否需要我去dubbogo/go-zookeeper那提个pr改一下这个小问题?

提一下吧

fixed;已在go-zookeeper侧,将invalidateWatcher改为原子读:dubbogo/go-zookeeper#12

@Alanxtl

Alanxtl commented Aug 20, 2026

Copy link
Copy Markdown
Member

执行make test-race时,TestCacheListenerRemoveListenerDropsWatchAtAutoLimitTestRestartCallBackResetsCache发生了数据竞争;原因是单测中执行到defer cluster.Stop()时,停止 ZooKeeper 服务端,接着Conn.loop检测到网络断开,触发的setStateremoveWatcher()中的invalidateWatcher()存在并发访问窗口,发生了state的读写竞争(前者是原子写,但后者却是普通读);由于后来reviewer提出要引入zk服务端依赖,这个数据竞争问题得以才暴露出来;
若要解决这个问题,可以在dubbogo/go-zookeeper中将invalidateWatcher的state读取改为已有的原子读取State()即可,是否需要我去dubbogo/go-zookeeper那提个pr改一下这个小问题?

提一下吧

fixed;已在go-zookeeper侧,将invalidateWatcher改为原子读:dubbogo/go-zookeeper#12

我发新tag了 go-zookeeper/v1.0.5 你更新到新版本吧

@ywxzm03

ywxzm03 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

执行make test-race时,TestCacheListenerRemoveListenerDropsWatchAtAutoLimitTestRestartCallBackResetsCache发生了数据竞争;原因是单测中执行到defer cluster.Stop()时,停止 ZooKeeper 服务端,接着Conn.loop检测到网络断开,触发的setStateremoveWatcher()中的invalidateWatcher()存在并发访问窗口,发生了state的读写竞争(前者是原子写,但后者却是普通读);由于后来reviewer提出要引入zk服务端依赖,这个数据竞争问题得以才暴露出来;
若要解决这个问题,可以在dubbogo/go-zookeeper中将invalidateWatcher的state读取改为已有的原子读取State()即可,是否需要我去dubbogo/go-zookeeper那提个pr改一下这个小问题?

提一下吧

fixed;已在go-zookeeper侧,将invalidateWatcher改为原子读:dubbogo/go-zookeeper#12

我发新tag了 go-zookeeper/v1.0.5 你更新到新版本吧

更新到1.0.5后,本地跑make test-race,gost编译失败:

undefined: zk.TestCluster
undefined: zk.StartTestCluster
undefined: zk.WithRetryTimes

原因是更新后的zk将部分测试api搬到了_test.go(WithRetryTimes是直接被删除了),所以这些api不会被编译到普通依赖包,而gost还用的之前的api,编译失败;如果要修这个,又要去gost那边将TestClusterStartTestClusterWithRetryTimesgo-zookeeper 转移到 gost 自己维护,我再去提个pr?

@Alanxtl

Alanxtl commented Aug 20, 2026

Copy link
Copy Markdown
Member

执行make test-race时,TestCacheListenerRemoveListenerDropsWatchAtAutoLimitTestRestartCallBackResetsCache发生了数据竞争;原因是单测中执行到defer cluster.Stop()时,停止 ZooKeeper 服务端,接着Conn.loop检测到网络断开,触发的setStateremoveWatcher()中的invalidateWatcher()存在并发访问窗口,发生了state的读写竞争(前者是原子写,但后者却是普通读);由于后来reviewer提出要引入zk服务端依赖,这个数据竞争问题得以才暴露出来;
若要解决这个问题,可以在dubbogo/go-zookeeper中将invalidateWatcher的state读取改为已有的原子读取State()即可,是否需要我去dubbogo/go-zookeeper那提个pr改一下这个小问题?

提一下吧

fixed;已在go-zookeeper侧,将invalidateWatcher改为原子读:dubbogo/go-zookeeper#12

我发新tag了 go-zookeeper/v1.0.5 你更新到新版本吧

更新到1.0.5后,本地跑make test-race,gost编译失败:

undefined: zk.TestCluster
undefined: zk.StartTestCluster
undefined: zk.WithRetryTimes

原因是更新后的zk将部分测试api搬到了_test.go(WithRetryTimes是直接被删除了),所以这些api不会被编译到普通依赖包,而gost还用的之前的api,编译失败;如果要修这个,又要去gost那边将TestClusterStartTestClusterWithRetryTimesgo-zookeeper 转移到 gost 自己维护,我再去提个pr?

ok 去 gost 也提一个pr吧

@ywxzm03

ywxzm03 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

执行make test-race时,TestCacheListenerRemoveListenerDropsWatchAtAutoLimitTestRestartCallBackResetsCache发生了数据竞争;原因是单测中执行到defer cluster.Stop()时,停止 ZooKeeper 服务端,接着Conn.loop检测到网络断开,触发的setStateremoveWatcher()中的invalidateWatcher()存在并发访问窗口,发生了state的读写竞争(前者是原子写,但后者却是普通读);由于后来reviewer提出要引入zk服务端依赖,这个数据竞争问题得以才暴露出来;
若要解决这个问题,可以在dubbogo/go-zookeeper中将invalidateWatcher的state读取改为已有的原子读取State()即可,是否需要我去dubbogo/go-zookeeper那提个pr改一下这个小问题?

提一下吧

fixed;已在go-zookeeper侧,将invalidateWatcher改为原子读:dubbogo/go-zookeeper#12

我发新tag了 go-zookeeper/v1.0.5 你更新到新版本吧

更新到1.0.5后,本地跑make test-race,gost编译失败:

undefined: zk.TestCluster
undefined: zk.StartTestCluster
undefined: zk.WithRetryTimes

原因是更新后的zk将部分测试api搬到了_test.go(WithRetryTimes是直接被删除了),所以这些api不会被编译到普通依赖包,而gost还用的之前的api,编译失败;如果要修这个,又要去gost那边将TestClusterStartTestClusterWithRetryTimesgo-zookeeper 转移到 gost 自己维护,我再去提个pr?

ok 去 gost 也提一个pr吧

gost已经将TestCluster、StartTestCluster、WithRetryTimes转移到本地维护了:dubbogo/gost#145

但在这之后,我尝试dubbogo依赖本地gost,重新跑了一遍make test-race,但etcd服务没连上导致测试超时,测试fail;原因是gost之前移除了grpc.WithBlock(),grpc.DialContext 不等待 Ready直接返回,超时没覆盖到后续Grant;所以我在另一个pr(https://github.com/dubbogo/gost/pull/146)中,在keepSession中,用独立的短超时 context发起首次Grant,且保留了gost删除WithBlock()的行为

@Alanxtl

Alanxtl commented Aug 22, 2026

Copy link
Copy Markdown
Member

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improve] zookeeper config-center GetProperties lacks in-memory read-through cache

5 participants