Skip to content

adding custom hash for all structs of compiled discovery chain#23393

Merged
panman90 merged 4 commits intomainfrom
compile_chain_hash
Mar 30, 2026
Merged

adding custom hash for all structs of compiled discovery chain#23393
panman90 merged 4 commits intomainfrom
compile_chain_hash

Conversation

@panman90
Copy link
Copy Markdown
Contributor

@panman90 panman90 commented Mar 27, 2026

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.

@panman90 panman90 marked this pull request as ready for review March 29, 2026 17:12
@panman90 panman90 requested review from a team as code owners March 29, 2026 17:12
@panman90 panman90 self-assigned this Mar 30, 2026
Copy link
Copy Markdown
Contributor

@anandmukul93 anandmukul93 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 ba1325d into main Mar 30, 2026
97 of 98 checks passed
rishabh-gupta-hashicorp pushed a commit that referenced this pull request Mar 31, 2026
* adding custom hash for all structs of compiled discovery chain

* adding test

* Adding changelog
@panman90 panman90 added backport/ent/1.18 Changes are backported to 1.18 ent backport/ent/1.21 changes are backported to 1.21 ent backport/ent/1.22 Changes are backported to 1.22 ent and removed pr/no-backport backport/ent/1.18 Changes are backported to 1.18 ent backport/ent/1.21 changes are backported to 1.21 ent backport/ent/1.22 Changes are backported to 1.22 ent labels Apr 1, 2026
@shani34 shani34 added backport/all Apply backports for all active releases per .release/versions.hcl and removed backport/ent/1.22 Changes are backported to 1.22 ent labels Apr 2, 2026
@hc-github-team-consul-core hc-github-team-consul-core added backport/1.22 Changes are backported to 1.22 backport/ent/1.18 Changes are backported to 1.18 ent backport/ent/1.21 changes are backported to 1.21 ent labels Apr 2, 2026
@hc-github-team-consul-core
Copy link
Copy Markdown
Collaborator

📣 Hi @panman90! a backport is missing for this PR [23393] for versions [1.18,1.21] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/all Apply backports for all active releases per .release/versions.hcl backport/ent/1.18 Changes are backported to 1.18 ent backport/ent/1.21 changes are backported to 1.21 ent backport/1.22 Changes are backported to 1.22

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants