Skip to content

Backport of adding custom hash for all structs of compiled discovery chain into release/1.22.x#23418

Merged
panman90 merged 3 commits into
release/1.22.xfrom
backport/compile_chain_hash/promptly-premium-tortoise
Apr 16, 2026
Merged

Backport of adding custom hash for all structs of compiled discovery chain into release/1.22.x#23418
panman90 merged 3 commits into
release/1.22.xfrom
backport/compile_chain_hash/promptly-premium-tortoise

Conversation

@hc-github-team-consul-core
Copy link
Copy Markdown
Collaborator

Backport

This PR is auto-generated from #23393 to be assessed for backporting due to the inclusion of the label backport/1.22.

The below text is copied from the body of the original PR.


Description

This PR removes the use of hashstructure_v2 ([github.com/mitchellh/hashstructure/v2] from compiled discovery chain hashing and replaces it with explicit custom hash implementations.

Instead of relying on generic reflective hashing, compiled discovery chain types now implement targeted appendHash logic that hashes each relevant field directly. This makes the hashing behavior more explicit, deterministic, and easier to reason about for performance-sensitive discovery-chain code.

The change includes custom hashing for [CompiledDiscoveryChain] and related nested types, with deterministic handling for:

  • Maps, by hashing entries in sorted key order
  • Slices, by preserving order where order is semantically meaningful
  • Optional nested values, by encoding both presence and nested hash state

Testing & Reproduction steps

Added/updated tests in respective test files to validate behaviour:

  • Determinism tests for map-backed fields.
  • Field-sensitivity tests to ensure meaningful field changes alter the hash.
  • Reflection-based hash coverage tests for structs that implement appendHash, so newly added fields fail tests if they are not included in the hash logic.

Performance validation
Captured pprof before vs after this change with 4000 watches on one service.
Comparison shows reduced time spent in hashing-related work on the compiled chain path, with corresponding CPU improvement in that scenario. Results are shared separately.

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.

  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.

  • If applicable, I've documented the impact of any changes to security controls.

    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.


Overview of commits

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Auto approved Consul Bot automated PR

@panman90 panman90 enabled auto-merge (squash) April 2, 2026 07:00
Copy link
Copy Markdown
Contributor

@kswap kswap left a comment

Choose a reason for hiding this comment

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

LGTM

@panman90 panman90 merged commit 82c1376 into release/1.22.x Apr 16, 2026
99 of 101 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants