dns_cache: test that zero failure backoff is floored at dns_min_refresh_rate#136
Draft
Copilot wants to merge 2 commits into
Draft
dns_cache: test that zero failure backoff is floored at dns_min_refresh_rate#136Copilot wants to merge 2 commits into
Copilot wants to merge 2 commits into
Conversation
…sh_rate Agent-Logs-Url: https://github.com/phlax/envoy/sessions/ba248f4d-3f4e-4efd-a832-57155165a8f1 Co-authored-by: phlax <454682+phlax@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add test for nextBackOffMs() == 0 case in DNS cache
dns_cache: test that zero failure backoff is floored at dns_min_refresh_rate
Apr 23, 2026
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.
Covers the
nextBackOffMs() == 0case of the failure-backoff floor added in envoyproxy#44570, as requested by @agrawroh.What the test does
random()→0beforeinitialize()so the jittered exponential backoff yields0on its first callhost_ttl=60s,dns_refresh_rate=60s,dns_min_refresh_rate=1s— the large TTL ensures the cap (host_ttl − elapsed) never engages, isolating the floor path1000ms(thedns_min_refresh_ratefloor), not0Placed immediately after the existing
ResolveFailureBackoffCappedByHostTtltest, which covers the complementary (cap) path.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
dl.google.com/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/bin/java bazel(envoy) --add-opens=java.base/java.lang=ALL-UNNAMED -Xverify:none -Djava.util.logging.config.file=/build/bazel_root/base/javalog.properties -Dcom.google.devtools.build.lib.util.LogHandlerQuerier.class=com.google.devtools.build.lib.util.SimpleLogHandler$HandlerQuerier -XX:-MaxFDLimit -Djava.library.path=/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib/server:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/ -Dfile.encoding=ISO-8859-1 -Duser.country= -Duser.language= -Duser.variant= -Xmx3g -DBAZEL_TRACK_SOURCE_DIRECTORIES=1 -Djavax.net.ssl.trustStore=/tmp/custom-cacerts -Djavax.net.ssl.trustStorePassword=changeit -jar /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/A-server.jar --max_idle_secs=10800 --noshutdown_on_low_sys_mem --connect_timeout_secs=30(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
Add one new test to
test/extensions/common/dynamic_forward_proxy/dns_cache_impl_test.ccthat covers thenextBackOffMs() == 0case of the failure-backoff floor in PR envoyproxy#44570. Reviewer @agrawroh asked for "a test with a seeded random generator that forcesnextBackOffMs() == 0".Push as a single commit on top of branch
flake-dfp-noin forkphlax/envoy. Do not open a new PR. Do not touch any other files.What the PR already does (on
flake-dfp-no)In
source/extensions/common/dynamic_forward_proxy/dns_cache_impl.ccfinishResolve, the failure branch now:primary_host_info->failure_backoff_strategy_->nextBackOffMs().host_ttl - elapsed(or sets to 0 if past TTL).min_refresh_interval_(dns_min_refresh_rate).So when the jittered exponential backoff returns 0, the armed interval should equal
min_refresh_interval_, not 0 and not 1ms.How the random generator is plumbed
In
DnsCacheImpl::DnsCacheImpltherandom_generator_member is initialised from:The backoff strategy is built in
PrimaryHostInfo::PrimaryHostInfoviaConfig::Utility::prepareDnsRefreshStrategy<...>(..., parent_.random_generator_).In the existing
DnsCacheImplTestfixture,context_is aNiceMock<Server::Configuration::MockGenericFactoryContext>whose server_context already carries aNiceMock<Random::MockRandomGenerator>accessible ascontext_.server_context_.api_.random_. We don't need to wire anything new — just set anON_CALLon itsrandom()method beforeinitialize().When
random()is stubbed to return0, the jittered exponential backoff returns0. (This is how jittered backoff works throughout Envoy tests: the jitter israndom() % next_interval, sorandom() == 0→ jitter 0; and with the base set to the configureddns_refresh_rate,nextBackOffMs()returns 0 on the first call because of the% next_intervalfolding whenrandom()is 0.)What to add
A new
TEST_F(DnsCacheImplTest, ResolveFailureBackoffZeroFlooredByMinRefresh)placed immediately after the existingResolveFailureBackoffCappedByHostTtltest in the same file.Shape should mirror
ResolveFailureBackoffCappedByHostTtlclosely — same fixture, sameInSequence s, same pattern ofMockTimers, sameloadDnsCacheEntry->resolve_cbflow.Steps:
Before
initialize(), stub the random generator:(Include
testing::Returnvia the existingusing testing::Return;or qualify.)Configure:
A 60s
host_ttl(well abovedns_min_refresh_rate) ensures the cap does not kick in and the test isolates the floor behaviour: raw backoff == 0, TTL cap leaves it at 0, floor lifts it tomin_refresh_interval_(1000ms).Standard first resolve:
loadDnsCacheEntry, success callback with one IP, assertonDnsHostAddOrUpdateandonLoadDnsCacheComplete, assert the first (success)enableTimeruses the successful path (this will bedns_ttlfrom the resolver response — copy the value from theResolveFailureBackoffCappedByHostTtltest's first-resolve block, i.e.EXPECT_CALL(*resolve_timer, enableTimer(std::chrono::milliseconds(60000), _));or whatever that test uses — keep it consistent).Advance sim time a small amount so
elapsedis well belowhost_ttl(e.g.simTime().advanceTimeWait(std::chrono::milliseconds(100));).Invoke the refresh timer to trigger a re-resolve, then set up the failure callback. Assertion:
This is the floor (`dns_min_refresh_ra...
This pull request was created from Copilot chat.