Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Flip runtime guard on use_fast_protobuf_hash #33121

Merged
merged 4 commits into from
Apr 1, 2024
Merged

Flip runtime guard on use_fast_protobuf_hash #33121

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
64a62d7
Flip runtime guard on fast_protobuf_hash
ravenblackx Mar 26, 2024
e8cea48
relnote
ravenblackx Mar 26, 2024
4ec41ca
Merge branch 'main' into fast_hash
ravenblackx Mar 26, 2024
a9e51b0
Merge branch 'main' into fast_hash
ravenblackx Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelogs/current.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ minor_behavior_changes:
Disables recvmmsg (multi-message) for reading packets from a client QUIC UDP socket, if GRO
is not set or not supported. recvmsg will be used instead. This behavior change can be
reverted by setting ``envoy.reloadable_features.disallow_quic_client_udp_mmsg`` to ``false``.
- area: config parsing, http cache filter
change: |
``envoy.restart_features.use_fast_protobuf_hash`` was flipped to true by default.
The expectation is that this will improve performance of the hash operation by 2x to 10x,
and reduce config update time by 10-25%. This change will also cause a one-time cache flush
for file_system_http_cache.

bug_fixes:
# *Changes expected to improve the state of the world and are unlikely to have negative effects*
Expand Down
3 changes: 1 addition & 2 deletions source/common/runtime/runtime_features.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ RUNTIME_GUARD(envoy_restart_features_allow_client_socket_creation_failure);
RUNTIME_GUARD(envoy_restart_features_quic_handle_certs_with_shared_tls_code);
RUNTIME_GUARD(envoy_restart_features_send_goaway_for_premature_rst_streams);
RUNTIME_GUARD(envoy_restart_features_udp_read_normalize_addresses);
RUNTIME_GUARD(envoy_restart_features_use_fast_protobuf_hash);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there is no UT to check default values, is that common?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are unit tests that test both true and false behaviors for this case - I made the unit tests force the values specifically so that changing the default wouldn't require updating the tests.
It may not be common, but I find it much less annoying than the other way.


// Begin false flags. Most of them should come with a TODO to flip true.

Expand Down Expand Up @@ -149,8 +150,6 @@ FALSE_RUNTIME_GUARD(envoy_restart_features_use_eds_cache_for_ads);
FALSE_RUNTIME_GUARD(envoy_reloadable_features_enable_universal_header_validator);
// TODO(pksohn): enable after fixing https://github.com/envoyproxy/envoy/issues/29930
FALSE_RUNTIME_GUARD(envoy_reloadable_features_quic_defer_logging_to_ack_listener);
// TODO(#31276): flip this to true after some test time.
FALSE_RUNTIME_GUARD(envoy_restart_features_use_fast_protobuf_hash);
// TODO(panting): flip this to true after some test time.
FALSE_RUNTIME_GUARD(envoy_reloadable_features_use_config_in_happy_eyeballs);

Expand Down
Loading