Skip to content

schema_registry/protobuf: fix MESSAGE_REMOVED for removed map fields#30400

Open
QueLLL wants to merge 1 commit intoredpanda-data:devfrom
QueLLL:sr/sr-protobuf-map-compat
Open

schema_registry/protobuf: fix MESSAGE_REMOVED for removed map fields#30400
QueLLL wants to merge 1 commit intoredpanda-data:devfrom
QueLLL:sr/sr-protobuf-map-compat

Conversation

@QueLLL
Copy link
Copy Markdown

@QueLLL QueLLL commented May 7, 2026

Protobuf map fields are represented in descriptors as compiler-generated
nested *Entry message types. Redpanda schema registry was comparing those map entry types
as regular nested messages during compatibility checks, so removing a map
field could incorrectly fail with MESSAGE_REMOVED

Skip protobuf map entry types when checking nested message compatibility.
This matches Confluent Schema Registry behavior and fixes map field removal
compatibility for all protobuf map key/value combinations.

Fixes #30374

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v26.1.x
  • v25.3.x
  • v25.2.x

Release Notes

Bug Fixes

  • Schema Registry: fixed protobuf compatibility checks incorrectly failing
    with MESSAGE_REMOVED when a map field is removed.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 7, 2026

CLA assistant check
All committers have signed the CLA.

@vbotbuildovich
Copy link
Copy Markdown
Collaborator

Retry command for Build#84210

please wait until all jobs are finished before running the slash command

/ci-repeat 1
skip-redpanda-build
skip-units
skip-rebase
tests/rptest/tests/cluster_linking_e2e_test.py::ShadowLinkingReplicationTests.test_with_restart@{"storage_mode":"tiered_cloud"}

@vbotbuildovich
Copy link
Copy Markdown
Collaborator

CI test results

test results on build#84210
test_status test_class test_method test_arguments test_kind job_url passed reason test_history
FLAKY(PASS) ShadowLinkingReplicationTests test_replication_with_failures {"storage_mode": "tiered_cloud"} integration https://buildkite.com/redpanda/redpanda/builds/84210#019e0676-51be-4294-a8d4-8b49ec5ed43b 19/21 Test PASSES after retries.No significant increase in flaky rate(baseline=0.0050, p0=0.0959, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3917, trust_threshold=0.5000) https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=ShadowLinkingReplicationTests&test_method=test_replication_with_failures
FLAKY(FAIL) ShadowLinkingReplicationTests test_with_restart {"storage_mode": "tiered_cloud"} integration https://buildkite.com/redpanda/redpanda/builds/84210#019e0678-8a4c-40ba-bf51-5f2209d0eabd 8/11 Test FAILS after retries.Significant increase in flaky rate(baseline=0.0079, p0=0.0027, reject_threshold=0.0100) https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=ShadowLinkingReplicationTests&test_method=test_with_restart
FLAKY(PASS) WriteCachingFailureInjectionE2ETest test_crash_all {"use_transactions": false} integration https://buildkite.com/redpanda/redpanda/builds/84210#019e0678-8a4d-4f20-aa35-d778ad017a69 9/11 Test PASSES after retries.No significant increase in flaky rate(baseline=0.1029, p0=0.6625, reject_threshold=0.0100. adj_baseline=0.2781, p1=0.1866, trust_threshold=0.5000) https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=WriteCachingFailureInjectionE2ETest&test_method=test_crash_all

nguyen-andrew
nguyen-andrew previously approved these changes May 8, 2026
Copy link
Copy Markdown
Member

@nguyen-andrew nguyen-andrew left a comment

Choose a reason for hiding this comment

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

Thanks for reporting and fixing this! LGTM

@nguyen-andrew
Copy link
Copy Markdown
Member

Looks like lint is failing, could you run bazel run //tools:clang_format and amend the existing commit? Thanks!

@nguyen-andrew nguyen-andrew dismissed their stale review May 8, 2026 23:19

Need to format

@QueLLL QueLLL force-pushed the sr/sr-protobuf-map-compat branch 2 times, most recently from 634777f to ae7d0a0 Compare May 9, 2026 08:34
@QueLLL QueLLL requested review from a team, kbatuigas and r-vasquez as code owners May 9, 2026 08:34
@QueLLL QueLLL requested review from ivotron and removed request for a team May 9, 2026 08:34
@QueLLL QueLLL force-pushed the sr/sr-protobuf-map-compat branch from ae7d0a0 to b80d657 Compare May 9, 2026 08:37
@QueLLL
Copy link
Copy Markdown
Author

QueLLL commented May 9, 2026

Looks like lint is failing, could you run bazel run //tools:clang_format and amend the existing commit? Thanks!

Done, thanks!

@QueLLL QueLLL force-pushed the sr/sr-protobuf-map-compat branch from b80d657 to c013b6f Compare May 9, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Schema Registry: protobuf map<string,string> field removal incompatible vs Confluent on v24/v25/v26 (ok on v23)

4 participants