dynamic_modules: remote source: add nack_on_cache_miss by kanurag94 · Pull Request #44044 · envoyproxy/envoy

kanurag94 · 2026-03-20T11:21:55Z

Commit Message: dynamic_modules: add nack_on_cache_miss for remote module sources

Additional Description:

This PR adds support for nacking a configuration if the dynamic module is not fetched and start a background fetch. The module may become available in the next config push and ready to use from cache.

Entries are keyed by sha256 so that multiple listeners can use the same fetch. The cache lives inside the factory and is safe to be used across listeners. Completed entries are cleaned up on the next createFilterFactory() and not during a fetch callback. So that the RemoteDataFetcher is not destroyed mid-fetch.

Risk Level: Low
Testing: Unit tests added. Manually validated
Docs Changes: Proto changes included
Release Notes: Added

[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>

repokitteh-read-only · 2026-03-20T11:22:01Z

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #44044 was opened by kanurag94.

see: more, trace.

Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>

kanurag94 · 2026-03-20T15:50:05Z

/retest

kanurag94 · 2026-03-23T10:12:57Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces the nack_on_cache_miss option for remote dynamic modules, enabling a non-blocking loading mechanism suitable for ECDS and per-route configurations. The changes are well-implemented, including clear documentation in the protobuf definition, a corresponding changelog entry, and a comprehensive set of tests that cover success, failure, and edge-case scenarios. The overall approach is sound. I have one minor suggestion to optimize a section of the code for better performance and readability.

Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>

repokitteh-read-only · 2026-03-23T11:29:38Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @wbpcode
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #44044 was ready_for_review by kanurag94.

see: more, trace.

wbpcode

/lgtm api

wbpcode · 2026-03-26T01:30:37Z

+void DynamicModuleConfigFactory::BackgroundFetchState::onSuccess(const std::string& data) {
+  auto result = Extensions::DynamicModules::newDynamicModuleFromBytes(data, sha256_, do_not_close_,
+                                                                      load_globally_);
+  if (!result.ok()) {
+    ENVOY_LOG(error, "Failed to load background-fetched module: {}", result.status().message());
+  } else {
+    ENVOY_LOG(info, "Background fetch complete, module cached for SHA256 {}", sha256_);
+  }
+  completed_ = true;
+}


I guess the background fetcher only need to fetch the data, validate its hash, and write it to local file. We needn't try to load it because it make no sense I think?

Make sense. It would have no real meaning. Thanks for catching this

wbpcode

I will say, great. Thanks for this great contribution and only two comments.

/wait

wbpcode · 2026-03-26T01:32:21Z

+  // This lives on the factory (rather than a singletonManager singleton) because
+  // REGISTER_FACTORY already makes the factory a process-lifetime singleton, so a
+  // separate registration would just be boilerplate for the same semantics.
+  // Entries are erased in createFilterFactory(), never inside a fetch callback,
+  // which means the RemoteDataFetcher is never destroyed while it's still running.
+  absl::flat_hash_map<std::string, std::unique_ptr<BackgroundFetchState>> background_fetches_;


I guess we can make this a singleton because in the future, we may want to support similar feature for other dynamic modules (like network filter dym, logger dym and so on)

sounds good!

Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>

wbpcode

LGTM. Thanks.

)  **Commit Message:** dynamic_modules: add nack_on_cache_miss for remote module sources **Additional Description:** This PR adds support for nacking a configuration if the dynamic module is not fetched and start a background fetch. The module may become available in the next config push and ready to use from cache. Entries are keyed by sha256 so that multiple listeners can use the same fetch. The cache lives inside the factory and is safe to be used across listeners. Completed entries are cleaned up on the next `createFilterFactory()` and not during a fetch callback. So that the `RemoteDataFetcher` is not destroyed mid-fetch. **Risk Leve**l: Low **Testing:** Unit tests added. Manually validated **Docs Changes**: Proto changes included **Release Notes:** Added [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> Signed-off-by: Xuyang Tao <taoxuy@google.com>

)  **Commit Message:** dynamic_modules: add nack_on_cache_miss for remote module sources **Additional Description:** This PR adds support for nacking a configuration if the dynamic module is not fetched and start a background fetch. The module may become available in the next config push and ready to use from cache. Entries are keyed by sha256 so that multiple listeners can use the same fetch. The cache lives inside the factory and is safe to be used across listeners. Completed entries are cleaned up on the next `createFilterFactory()` and not during a fetch callback. So that the `RemoteDataFetcher` is not destroyed mid-fetch. **Risk Leve**l: Low **Testing:** Unit tests added. Manually validated **Docs Changes**: Proto changes included **Release Notes:** Added [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> Signed-off-by: Jonathan Wu <jtwu@google.com>

)  **Commit Message:** dynamic_modules: add nack_on_cache_miss for remote module sources **Additional Description:** This PR adds support for nacking a configuration if the dynamic module is not fetched and start a background fetch. The module may become available in the next config push and ready to use from cache. Entries are keyed by sha256 so that multiple listeners can use the same fetch. The cache lives inside the factory and is safe to be used across listeners. Completed entries are cleaned up on the next `createFilterFactory()` and not during a fetch callback. So that the `RemoteDataFetcher` is not destroyed mid-fetch. **Risk Leve**l: Low **Testing:** Unit tests added. Manually validated **Docs Changes**: Proto changes included **Release Notes:** Added [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> Signed-off-by: Nick Shipilov <nick.shipilov.n@gmail.com>

)  **Commit Message:** dynamic_modules: add nack_on_cache_miss for remote module sources **Additional Description:** This PR adds support for nacking a configuration if the dynamic module is not fetched and start a background fetch. The module may become available in the next config push and ready to use from cache. Entries are keyed by sha256 so that multiple listeners can use the same fetch. The cache lives inside the factory and is safe to be used across listeners. Completed entries are cleaned up on the next `createFilterFactory()` and not during a fetch callback. So that the `RemoteDataFetcher` is not destroyed mid-fetch. **Risk Leve**l: Low **Testing:** Unit tests added. Manually validated **Docs Changes**: Proto changes included **Release Notes:** Added [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>

dynamic_modules: remote source: add nack_on_cache_miss

915948f

Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>

make format

6be44b8

Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>

gemini-code-assist bot reviewed Mar 23, 2026

View reviewed changes

Comment thread source/extensions/filters/http/dynamic_modules/factory.cc

repokitteh-read-only bot added the api label Mar 23, 2026

gemini review addressed

644b177

Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>

kanurag94 marked this pull request as ready for review March 23, 2026 11:29

kanurag94 requested review from agrawroh, mathetake, mattklein123 and wbpcode as code owners March 23, 2026 11:29

repokitteh-read-only bot assigned wbpcode Mar 23, 2026

wbpcode reviewed Mar 25, 2026

View reviewed changes

repokitteh-read-only bot removed the api label Mar 25, 2026

wbpcode reviewed Mar 26, 2026

View reviewed changes

repokitteh-read-only bot added the waiting label Mar 26, 2026

review: make fetcher a singleton and only write to disk

ca403b2

Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>

repokitteh-read-only bot removed the waiting label Mar 26, 2026

kanurag94 requested a review from wbpcode March 26, 2026 06:30

wbpcode approved these changes Mar 30, 2026

View reviewed changes

wbpcode merged commit d7435f9 into envoyproxy:main Mar 30, 2026
30 checks passed

Conversation

kanurag94 commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Mar 20, 2026

Uh oh!

kanurag94 commented Mar 20, 2026

Uh oh!

kanurag94 commented Mar 23, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

repokitteh-read-only bot commented Mar 23, 2026

Uh oh!

wbpcode left a comment

Choose a reason for hiding this comment

Uh oh!

wbpcode Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

kanurag94 Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

wbpcode left a comment

Choose a reason for hiding this comment

Uh oh!

wbpcode Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

kanurag94 Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

wbpcode left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kanurag94 commented Mar 20, 2026 •

edited

Loading