Skip to content

Feat/revision canonical metadata#3370

Open
Qiao-yq wants to merge 8 commits into
apache:developfrom
Qiao-yq:feat/revision-canonical-metadata
Open

Feat/revision canonical metadata#3370
Qiao-yq wants to merge 8 commits into
apache:developfrom
Qiao-yq:feat/revision-canonical-metadata

Conversation

@Qiao-yq
Copy link
Copy Markdown
Contributor

@Qiao-yq Qiao-yq commented Jun 4, 2026

Description

Fixes #3350

Summary

Replace the CRC32-based revision calculation with a canonical SHA512-based approach.

The existing resolveRevision builds revision from a few raw URL fields (app + path + version + port + method) and sums CRC32 values — missing group, protocol, and params (timeout/loadbalance/cluster/serialization, etc.), with weak collision resistance and no binding to the actual serialized metadata content.

Changes

metadata/info/metadata_info.go
Three new methods:

  • ServiceInfo.toDescString() — deterministic string representation in format name|group|version|protocol|port|path|params|methods, with sorted params keys and sorted methods for stable output.
  • CalRevision(app, services) — computes SHA512 hex digest from app concatenated with sorted ServiceInfo.toDescString() outputs. Returns "0" for empty services.
  • MetadataInfo.CalAndGetRevision() — updates info.Revision in-place from its own canonical services map.

registry/servicediscovery/customizer/service_revision_customizer.go

  • resolveRevision — rewrites the core logic: instead of CRC32‑summing app+path+version+port+method, it converts each URL to a canonical ServiceInfo via NewServiceInfoWithURL, groups by MatchKey, and delegates to info.CalRevision.
  • URL source — customizers now obtain URLs from instance.GetServiceMetadata() (instance‑scoped) rather than GetMetadataService().GetExportedServiceURLs() (global aggregation). Each instance's revision now correctly reflects only its own app's metadata.

registry/servicediscovery/customizer/service_revision_customizer_test.go (new)
10 test cases covering: group/protocol/params/methods/version change detection, deterministic output, empty input, URL ordering independence, param key ordering independence, and non‑IncludeKeys filtering.

Compatibility

The revision algorithm change is a one-time switch. On the consumer side,
GetMetadataInfo() fetches metadata by revision:

  1. Check local disk cache by revision key.
  2. If remote, query the metadata center (Nacos/ZK/Etcd). Failure →
    skip the instance.
    No automatic fallback to RPC.
  3. If local, call the provider's MetadataService directly via RPC.

When multiple instances share the same revision, OnEvent retries them
sequentially until one succeeds. Consumers using a stale cached revision
will miss transiently, then pick up the new revision from the next
service discovery event.

Checklist

  • I confirm the target branch is develop
  • Code has passed local testing
  • I have added tests that prove my fix is effective or that my feature works

Qiao-yq added 2 commits June 4, 2026 17:10
Add three methods to replace CRC32-based revision with MD5 over
deterministic ServiceInfo serialization, aligning with Java dubbo's
MetadataInfo.calAndGetRevision().
- ServiceInfo.toDescString(): deterministic string representation
  in format (name|group|version|protocol|port|path|params|methods),
  with sorted params/methods keys for stable output.
- CalRevision(app, services): computes MD5 hex digest from app name
  concatenated with sorted ServiceInfo.toDescString() outputs.
  Returns 0 for empty services (matches Java EMPTY_REVISION).
- MetadataInfo.CalAndGetRevision(): updates info.Revision in-place.
This replaces the coarse app+path+version+port+method CRC32 approach
in resolveRevision, ensuring group/protocol/params changes are captured
and revision is strongly bound to the serialized MetadataInfo content.
…alculation

Refactor resolveRevision to derive revision from canonical ServiceInfo
objects rather than raw URLs, aligning with Java dubbo's
MetadataInfo.calAndGetRevision().

Changes in service_revision_customizer.go:
- resolveRevision now converts URLs to ServiceInfo via NewServiceInfoWithURL,
  groups by MatchKey, and delegates to info.CalRevision (MD5 over sorted
  toDescString) instead of CRC32-summing app+path+version+port+method.
- Customizers now source URLs from instance.GetServiceMetadata() rather
  than the global GetMetadataService(), so each instance's revision is
  scoped to its own MetadataInfo. This fixes cross-test state leakage.

Changes in service_revision_customizer_test.go (new, 10 cases):
- Group / protocol / params / methods / version change triggers revision change
- Deterministic: same input always produces same revision
- Empty URL list returns 0
- Ordering-independent: URL insertion order and param key order do not
  affect revision
- Non-IncludeKeys params are ignored
Qiao-yq added 2 commits June 4, 2026 19:47
Switch the hash algorithm used in CalRevision from MD5 (32 hex chars)
to SHA-512 (128 hex chars) for stronger collision resistance.
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 4, 2026

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.61%. Comparing base (60d1c2a) to head (2c86207).
⚠️ Report is 816 commits behind head on develop.

Files with missing lines Patch % Lines
...iscovery/customizer/service_revision_customizer.go 46.66% 8 Missing ⚠️
metadata/info/metadata_info.go 92.00% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3370      +/-   ##
===========================================
+ Coverage    46.76%   52.61%   +5.85%     
===========================================
  Files          295      492     +197     
  Lines        17172    37916   +20744     
===========================================
+ Hits          8031    19951   +11920     
- Misses        8287    16357    +8070     
- Partials       854     1608     +754     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@Alanxtl Alanxtl left a comment

Choose a reason for hiding this comment

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

PR 描述里说 remote cache miss 会 fallback 到 provider metadata service,但代码 service_instances_changed_listener_impl.go:261-268 没有 fallback:remote metadata report 失败就直接返回 error,实例会被跳过。这个兼容性说明需要改,或者补 fallback 和端到端测试。

Comment thread metadata/info/metadata_info.go
@Alanxtl
Copy link
Copy Markdown
Contributor

Alanxtl commented Jun 5, 2026

另外pr描述里面改一下 我们没有align with java

Environment is instance-level routing metadata (per the consumer-side
comment in service_instances_changed_listener_impl.go). The consumer
always overrides it from the current instance metadata, regardless of
what is cached by revision. Including it in IncludeKeys caused revision
to change between instances with different environment tags, producing
spurious revision updates with no routing impact.
Comment thread metadata/info/metadata_info.go Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates dubbo-go’s application-level metadata revision calculation to be deterministic and collision-resistant by switching from a CRC32-sum over a subset of URL fields to a canonical SHA-512 digest over a stable, sorted service description.

Changes:

  • Introduce canonical ServiceInfo string serialization and compute revision via SHA-512 over app + sorted(services).
  • Rework service revision customizers to compute revision from instance-scoped ServiceMetadata URLs rather than a globally aggregated URL set.
  • Add a dedicated test suite covering revision change detection and determinism across ordering and filtering cases.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
registry/servicediscovery/customizer/service_revision_customizer.go Switch revision computation to canonical ServiceInfo + SHA-512 and use instance service metadata as the URL source.
registry/servicediscovery/customizer/service_revision_customizer_test.go Add test coverage for revision determinism and change detection across key metadata dimensions.
metadata/info/metadata_info.go Add canonical serialization (toDescString), SHA-512 CalRevision, and MetadataInfo.CalAndGetRevision.
metadata/info/metadata_info_test.go Update expectations around instance-level params (e.g., environment) being excluded from ServiceInfo params.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread metadata/info/metadata_info.go Outdated
Comment thread metadata/info/metadata_info.go
Copy link
Copy Markdown
Contributor

@Alanxtl Alanxtl left a comment

Choose a reason for hiding this comment

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

lgtm

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 6, 2026

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Improve application-level metadata revision calculation / 改进应用级 metadata revision 计算

4 participants