ETCD 接口暴露#47
Closed
yousongyang wants to merge 1 commit into
Closed
Conversation
There was a problem hiding this comment.
Pull request overview
This PR focuses on exposing and refactoring parts of the ETCD module/watchers API, including moving watcher callback wrapper types into the public header and centralizing watcher configuration wiring.
Changes:
- Refactored
etcd_module::init()startup keepalive validation into a new helper (check_keepalive_actor_start_failed). - Added
etcd_watcher::fill_conf_from_protobuf(...)to consolidate watcher configuration setup from protobuf config. - Exposed additional ETCD-related helper APIs (path generation, wrapper factories, callback list accessors) from
etcd_module.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/atframe/modules/etcd_module.cpp | Refactors init keepalive startup checking; switches watcher setup to fill_conf_from_protobuf; adds new exported helper accessors/factories. |
| include/atframe/modules/etcd_module.h | Moves callback wrapper structs into the header and adds new exported APIs for wrapper creation and callback list access. |
| src/atframe/etcdcli/etcd_watcher.cpp | Implements fill_conf_from_protobuf and adds protobuf includes/util for duration conversion. |
| include/atframe/etcdcli/etcd_watcher.h | Forward-declares watcher protobuf config type and declares fill_conf_from_protobuf. |
Comment on lines
275
to
285
| bool etcd_module::check_keepalive_actor_start_failed(const std::list<etcd_keepalive::ptr_t> &keepalives) { | ||
| // setup timer for timeout | ||
| uv_timer_t tick_timer; | ||
| init_timer_data_type tick_timer_data; | ||
| uv_timer_init(get_app()->get_bus_node()->get_evloop(), &tick_timer); | ||
| tick_timer_data.cluster = &etcd_ctx_; | ||
| tick_timer_data.timeout = std::chrono::system_clock::now(); | ||
| tick_timer_data.timeout += std::chrono::seconds(conf.init().timeout().seconds()); | ||
| tick_timer_data.timeout += std::chrono::seconds(get_configure().init().timeout().seconds()); | ||
| tick_timer_data.timeout += std::chrono::duration_cast<std::chrono::system_clock::duration>( | ||
| std::chrono::nanoseconds(conf.init().timeout().nanos())); | ||
| std::chrono::nanoseconds(get_configure().init().timeout().nanos())); | ||
| tick_timer.data = &tick_timer_data; |
owent
requested changes
May 7, 2026
| using topology_info_event_callback_list_t = std::list<topology_info_event_callback_t>; | ||
| using topology_info_event_callback_handle_t = topology_info_event_callback_list_t::iterator; | ||
|
|
||
| struct LIBATAPP_MACRO_API_HEAD_ONLY topology_watcher_callback_list_wrapper_t { |
Owner
There was a problem hiding this comment.
不要对外暴露细节,如果需要包一层类型。导出类型要显式声明导出 拷贝构造、移动构造、拷贝复制、移动复制。
| LIBATAPP_MACRO_NAMESPACE_BEGIN | ||
|
|
||
| namespace { | ||
| static std::chrono::system_clock::duration convert_to_chrono(const ATBUS_MACRO_PROTOBUF_NAMESPACE_ID::Duration &in, |
| LIBATAPP_MACRO_API topology_watcher_callback_list_wrapper_t | ||
| create_topology_watcher_callback_list_wrapper(std::list<topology_watcher_list_callback_t> &cbks); | ||
|
|
||
| LIBATAPP_MACRO_API std::list<topology_watcher_list_callback_t> &get_topology_watcher_callbacks() noexcept; |
Owner
There was a problem hiding this comment.
禁止暴露底层存储结构,必须通过handle操作,handle必须考虑容器失效或迭代器无效。
|
|
||
| // Setup for first, we must check if all resource available. | ||
| bool is_failed = false; | ||
| bool is_failed = check_keepalive_actor_start_failed(internal_discovery_keepalive_actors_) || |
| return rpc_.startup_random_delay_max; | ||
| } | ||
|
|
||
| LIBATAPP_MACRO_API void fill_conf_from_protobuf( |
Comment on lines
398
to
403
| // close timer for timeout | ||
| uv_timer_stop(&tick_timer); | ||
| uv_close(reinterpret_cast<uv_handle_t *>(&tick_timer), init_timer_closed_callback); | ||
| while (tick_timer.data != nullptr) { | ||
| uv_run(get_app()->get_bus_node()->get_evloop(), UV_RUN_ONCE); | ||
| } |
owent
requested changes
May 7, 2026
| tick_timer_data.cluster = &etcd_ctx_; | ||
| tick_timer_data.timeout = std::chrono::system_clock::now(); | ||
| tick_timer_data.timeout += std::chrono::seconds(conf.init().timeout().seconds()); | ||
| tick_timer_data.timeout += std::chrono::seconds(get_configure().init().timeout().seconds()); |
|
|
||
| LIBATAPP_MACRO_API void parse_timepoint(gsl::string_view in, ATBUS_MACRO_PROTOBUF_NAMESPACE_ID::Timestamp &out); | ||
| LIBATAPP_MACRO_API void parse_duration(gsl::string_view in, ATBUS_MACRO_PROTOBUF_NAMESPACE_ID::Duration &out); | ||
| LIBATAPP_MACRO_API std::chrono::system_clock::duration convert_to_chrono( |
Owner
There was a problem hiding this comment.
全局接口名称要包含模块信息,按下面通用接口一样使用模板适配更多类型。
补全单元测试。
| LIBATAPP_MACRO_API void set_maybe_update_keepalive_discovery_metadata(); | ||
|
|
||
| // Return True If Failed | ||
| LIBATAPP_MACRO_API bool check_keepalive_actor_start_failed( |
Comment on lines
409
to
412
| uv_close(reinterpret_cast<uv_handle_t *>(&tick_timer), init_timer_closed_callback); | ||
| while (tick_timer.data != nullptr) { | ||
| uv_run(get_app()->get_bus_node()->get_evloop(), UV_RUN_ONCE); | ||
| } |
Comment on lines
+160
to
+163
| // Return True If Failed | ||
| LIBATAPP_MACRO_API bool check_keepalive_actor_start_failed( | ||
| const std::vector<const std::list<etcd_keepalive::ptr_t> *> &keepalive_actors); | ||
|
|
Comment on lines
186
to
189
| LIBATAPP_MACRO_API const atapp::protocol::atapp_etcd &get_configure() const; | ||
| LIBATAPP_MACRO_API const std::string &get_configure_path() const; | ||
| LIBATAPP_MACRO_API std::string gen_etcd_path(const std::string &path) const; | ||
|
|
Comment on lines
+19
to
+20
| #include <memory/rc_ptr.h> | ||
|
|
owent
reviewed
May 7, 2026
owent
requested changes
May 7, 2026
4dfb592 to
005050b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.