deardeng commented on code in PR #69732:
URL: https://github.com/apache/doris/pull/69732#discussion_r4399019082
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -138,6 +138,20 @@ std::string get_instance_id(const
std::shared_ptr<ResourceManager>& rc_mgr,
LOG(WARNING) << msg << ", cloud_unique_id=" << cloud_unique_id;
return "";
}
+ // Anyone knowing the instance_id can build a degraded format
cloud_unique_id, so it
+ // is only trusted when it is a registered node of that instance.
+ bool registered = false;
+ for (const auto& cluster : instance.clusters()) {
+ for (const auto& node : cluster.nodes()) {
+ registered |= node.cloud_unique_id() == cloud_unique_id;
+ }
+ }
+ if (!registered) {
Review Comment:
Thanks, the mechanism is right: on a kv error other than TXN_KEY_NOT_FOUND,
get_instance returns before parsing, so the instance is empty and the request
is rejected, with a log wrongly saying the node is not registered.
Rejecting is intended. The degraded path is reached only when the
cloud_unique_id misses the node cache, and its membership must be read from kv
before it is trusted. Accepting on a failed read would let a forged
cloud_unique_id through, which is what this PR fixes. A legitimate request is
rejected only if both happen: its node is not in this meta service's cache yet
(e.g. just added through another meta service, before the refresh notification
arrives), and that single instance read fails.
Making it retryable needs get_instance_id to return the kv error and all 74
callers to map it, which is a larger refactor than this fix, so it is left out
of this PR.
2d895c7e693 handles a failed read explicitly: still rejected, and the log
carries the kv error. GetInstanceIdKvErrorTest covers it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
