github-actions[bot] commented on code in PR #63363: URL: https://github.com/apache/doris/pull/63363#discussion_r3687585256
########## be/test/util/dns_cache_test.cpp: ########## @@ -0,0 +1,592 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "util/dns_cache.h" + +#include <gtest/gtest-message.h> +#include <gtest/gtest-test-part.h> + +#include "common/config.h" +#include "gtest/gtest_pred_impl.h" + +namespace doris { + +class DNSCacheTest : public testing::Test { +public: + DNSCacheTest() = default; + ~DNSCacheTest() override = default; + +protected: + void SetUp() override { + _saved_threshold = config::dns_cache_max_consecutive_failures; + _saved_log_every = config::dns_cache_log_every_n_failures; + _saved_negative_ttl = config::dns_cache_negative_ttl_seconds; + } + + void TearDown() override { + config::dns_cache_max_consecutive_failures = _saved_threshold; + config::dns_cache_log_every_n_failures = _saved_log_every; + config::dns_cache_negative_ttl_seconds = _saved_negative_ttl; + } + + // Build a resolver whose failure behaviour can be flipped at runtime. + static DNSCache::Resolver make_resolver(bool* should_fail, const std::string& ip = "1.2.3.4") { + return [should_fail, ip](const std::string&, std::string& out, bool) -> Status { + if (*should_fail) { + return Status::InternalError("mock failure"); + } + out = ip; + return Status::OK(); + }; + } + +private: + int32_t _saved_threshold = 0; + int32_t _saved_log_every = 0; + int32_t _saved_negative_ttl = 0; +}; + +// ── existing tests ──────────────────────────────────────────────────────────── + +// Sanity: localhost resolves successfully and is cached. +TEST_F(DNSCacheTest, resolve_localhost) { + DNSCache cache; + std::string ip; + EXPECT_TRUE(cache.get("localhost", &ip).ok()); + EXPECT_FALSE(ip.empty()); + // Second call hits the cache fast path and returns the same IP. + std::string ip2; + EXPECT_TRUE(cache.get("localhost", &ip2).ok()); + EXPECT_EQ(ip, ip2); + EXPECT_EQ(1u, cache.size_for_test()); +} + +// Unresolvable hostname on first access returns InternalError and is NOT cached. +TEST_F(DNSCacheTest, first_miss_does_not_cache) { + DNSCache cache; + std::string ip; + Status st = cache.get("this-host-does-not-exist.invalid", &ip); + EXPECT_FALSE(st.ok()); + EXPECT_EQ(0u, cache.size_for_test()); +} + +// Repeated successful resolution does not grow the cache and does not accumulate +// any failure state. +TEST_F(DNSCacheTest, success_keeps_cache_stable) { + DNSCache cache; + std::string ip; + for (int i = 0; i < 8; ++i) { + EXPECT_TRUE(cache.get("localhost", &ip).ok()); + } + EXPECT_EQ(1u, cache.size_for_test()); +} + +// The eviction config can be disabled by setting threshold <= 0 (legacy behavior). +// In legacy mode, an unresolvable hostname on first access is still not cached, +// matching the pre-fix semantics for callers. +TEST_F(DNSCacheTest, eviction_disabled_when_threshold_zero) { Review Comment: This test never reaches the code controlled by `dns_cache_max_consecutive_failures`: a first miss for a never-cached `.invalid` host fails before `_refresh_once()` reads the threshold, so it would still pass if the `threshold > 0` guard regressed and cached hosts were evicted in the documented legacy mode. Please populate a host with the injected resolver, switch it to failure, run multiple `refresh_for_test()` cycles with the threshold set to zero, and assert that the cached host/IP remains available. -- 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]
