Skip to content

Commit e8d02aa

Browse files
cluster_link: apply the unsupported-feature policy on schema import
The reconciler now reads the link's configured policy and acts on the unsupported fields a source schema carries. REMOVE writes only the supported projection, counts the removed fields, and logs them. FAIL treats an unsupported schema as a per-item failure: it logs the offending fields, counts an error, and skips that schema while the rest of the subjects still replicate. If unset, the policy defaults to FAIL. FAIL deliberately does not abort the whole sync -- only global errors like source unavailability fail-fast. This collects all per-item errors and keeps replicating whatever the destination can store, which is more useful for a migration than stopping on the first offender. The unsupported detail (subject/version and which fields) is written to the broker log; the error count flows through reconcile_stats into the sync summary. REMOVE is counted at the single per-node import point (once, even on the 2-fetch path), and its count reaches unsupported_features_removed in the sync summary (current sync and totals), already surfaced by rpk shadow status. The source reader stays policy-agnostic.
1 parent 62c1d54 commit e8d02aa

8 files changed

Lines changed: 298 additions & 20 deletions

File tree

src/v/cluster_link/schema_registry_sync/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ redpanda_cc_library(
7676
"//src/v/ssx:future_util",
7777
"//src/v/ssx:sformat",
7878
"@abseil-cpp//absl/container:flat_hash_set",
79+
"@fmt",
7980
],
8081
visibility = ["//visibility:public"],
8182
deps = [

src/v/cluster_link/schema_registry_sync/mirroring_task.cc

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,13 @@ model::task_status_report mirroring_task::get_status_report() const {
215215
status.current_sync->summary.subject_versions_changed
216216
+= _reconcile_stats.versions_changed;
217217
status.current_sync->summary.errors += _reconcile_stats.errors;
218+
status.current_sync->summary.unsupported_features_removed
219+
+= _reconcile_stats.unsupported_features_removed;
218220
status.totals_since_task_start.subject_versions_changed
219221
+= _reconcile_stats.versions_changed;
220222
status.totals_since_task_start.errors += _reconcile_stats.errors;
223+
status.totals_since_task_start.unsupported_features_removed
224+
+= _reconcile_stats.unsupported_features_removed;
221225
}
222226
report.detail = model::task_detail{
223227
.schema_registry_sync_status = std::move(status)};
@@ -499,6 +503,7 @@ ss::future<> mirroring_task::list_one_subject(
499503
ss::future<task::state_transition> mirroring_task::full_source_sync(
500504
ss::abort_source& as,
501505
const chunked_hash_set<ppsr::context>& contexts,
506+
model::schema_registry_sync_config::unsupported_feature_policy feature_policy,
502507
const ss::noncopyable_function<bool(const ppsr::context_subject&)>&
503508
in_scope) {
504509
// Cluster-global, so safe to read mid-sync (unlike the per-link config a
@@ -624,7 +629,8 @@ ss::future<task::state_transition> mirroring_task::full_source_sync(
624629
_destination,
625630
[&in_scope](const ppsr::context_subject& cs) { return in_scope(cs); },
626631
_mapper,
627-
limits};
632+
limits,
633+
feature_policy};
628634

629635
// The reconciler increments _reconcile_stats live (reflected mid-sync by
630636
// get_status_report); the fold below moves them into persistent state.
@@ -655,9 +661,13 @@ ss::future<task::state_transition> mirroring_task::full_source_sync(
655661
_status.current_sync->summary.subject_versions_changed
656662
+= stats.versions_changed;
657663
_status.current_sync->summary.errors += stats.errors;
664+
_status.current_sync->summary.unsupported_features_removed
665+
+= stats.unsupported_features_removed;
658666
_status.totals_since_task_start.subject_versions_changed
659667
+= stats.versions_changed;
660668
_status.totals_since_task_start.errors += stats.errors;
669+
_status.totals_since_task_start.unsupported_features_removed
670+
+= stats.unsupported_features_removed;
661671

662672
const auto purged = co_await purge_destination_only_versions(
663673
std::move(to_purge), as);
@@ -826,9 +836,13 @@ mirroring_task::run_impl(ss::abort_source& as) {
826836
// phase shares one mapping without re-reading _config (which a concurrent
827837
// update_config could swap mid-run).
828838
_mapper = context_mapper::make(_config);
839+
// Snapshot the per-link policy while `api` is known non-null. A concurrent
840+
// update_config can swap `_config` across the co_awaits below (and inside
841+
// full_source_sync), so it must not re-read api_mode() after suspending.
842+
const auto feature_policy = api->feature_policy;
829843

830844
co_await refresh_destination_inventory(in_scope, as);
831-
co_return co_await full_source_sync(as, contexts, in_scope);
845+
co_return co_await full_source_sync(as, contexts, feature_policy, in_scope);
832846
}
833847

834848
task::state_transition

src/v/cluster_link/schema_registry_sync/mirroring_task.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ class mirroring_task : public task {
123123
ss::future<state_transition> full_source_sync(
124124
ss::abort_source&,
125125
const chunked_hash_set<ppsr::context>& contexts,
126+
model::schema_registry_sync_config::unsupported_feature_policy
127+
feature_policy,
126128
const ss::noncopyable_function<bool(const ppsr::context_subject&)>&
127129
in_scope);
128130

src/v/cluster_link/schema_registry_sync/reconciler.cc

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include <seastar/coroutine/as_future.hh>
2323
#include <seastar/util/defer.hh>
2424

25+
#include <fmt/ranges.h>
26+
2527
#include <algorithm>
2628
#include <array>
2729
#include <chrono>
@@ -110,12 +112,14 @@ reconciler::reconciler(
110112
schema::registry* destination,
111113
ss::noncopyable_function<bool(const ppsr::context_subject&)> in_scope,
112114
const context_mapper& mapper,
113-
limits lim)
115+
limits lim,
116+
model::schema_registry_sync_config::unsupported_feature_policy feature_policy)
114117
: _source(source)
115118
, _destination(destination)
116119
, _in_scope(std::move(in_scope))
117120
, _mapper(&mapper)
118121
, _limits(lim)
122+
, _feature_policy(feature_policy)
119123
, _mem(std::max<size_t>(1, lim.memory_bytes), "schema_registry_sync/memory") {
120124
// Floor the budget so the clamp `min(body_size, memory_bytes)` and the
121125
// semaphore agree; a degenerate 0 admits one over-budget body at a time.
@@ -138,6 +142,7 @@ ss::future<source_result<void>> reconciler::reconcile(
138142
_stats = &stats;
139143
_stats->versions_changed = 0;
140144
_stats->errors = 0;
145+
_stats->unsupported_features_removed = 0;
141146
_outstanding = 0;
142147
_done = false;
143148
_fault.reset();
@@ -316,6 +321,12 @@ reconciler::discover(const ppsr::subject_version& n, ss::abort_source& as) {
316321
co_return source_result<void>{};
317322
}
318323

324+
// Reject under FAIL before queuing references, so a schema we are going to
325+
// reject does not drag its dependency subtree into the destination.
326+
if (fail_if_contains_unsupported(n, fetched.value())) {
327+
co_return source_result<void>{};
328+
}
329+
319330
// The body is released here (fetched goes out of scope): the node will be
320331
// re-fetched once its references complete, leading to it being fetched
321332
// twice in this case; and at most twice on average overall.
@@ -375,8 +386,56 @@ reconciler::do_import(const ppsr::subject_version& n, ss::abort_source& as) {
375386
co_return source_result<void>{};
376387
}
377388

389+
bool reconciler::fail_if_contains_unsupported(
390+
const ppsr::subject_version& n, const ppsr::source_schema_read& read) {
391+
if (
392+
_feature_policy
393+
!= model::schema_registry_sync_config::unsupported_feature_policy::fail
394+
|| read.unsupported.empty()) {
395+
return false;
396+
}
397+
// The FAIL policy refuses to import a schema the destination cannot store
398+
// faithfully, but this is a per-item failure, not a whole-sync abort: count
399+
// it, log the offending fields, and let the rest of the subjects replicate.
400+
vlog(
401+
cllog.warn,
402+
"{}/{} carries {} unsupported schema feature(s) the destination cannot "
403+
"store; failing it under the FAIL policy: {}",
404+
n.sub,
405+
n.version,
406+
read.unsupported.size(),
407+
fmt::join(read.unsupported, ", "));
408+
fail(n);
409+
return true;
410+
}
411+
412+
void reconciler::count_if_contains_unsupported_removed(
413+
const ppsr::subject_version& n, const ppsr::source_schema_read& read) {
414+
if (
415+
_feature_policy
416+
!= model::schema_registry_sync_config::unsupported_feature_policy::
417+
remove
418+
|| read.unsupported.empty()) {
419+
return;
420+
}
421+
vlog(
422+
cllog.info,
423+
"Removed {} unsupported schema feature(s) from {}/{}: {}",
424+
read.unsupported.size(),
425+
n.sub,
426+
n.version,
427+
fmt::join(read.unsupported, ", "));
428+
_stats->unsupported_features_removed += read.unsupported.size();
429+
}
430+
378431
ss::future<bool> reconciler::import_body(
379432
const ppsr::subject_version& n, ppsr::source_schema_read read) {
433+
// Authoritative policy gate, next to the REMOVE handling below: every
434+
// import path funnels through here (discover also rejects early, before
435+
// queuing refs).
436+
if (fail_if_contains_unsupported(n, read)) {
437+
co_return false;
438+
}
380439
data(n).state = node_state::importing;
381440
// The graph key `n` stays in the source namespace; only the schema written
382441
// to the destination is remapped.
@@ -414,6 +473,10 @@ ss::future<bool> reconciler::import_body(
414473
}
415474
data(n).state = node_state::done;
416475
++_stats->versions_changed;
476+
// Account for REMOVE-stripped features only after the projection actually
477+
// lands on the destination, so a failed import does not report features as
478+
// removed. read.schema is moved-from above, but read.unsupported is intact.
479+
count_if_contains_unsupported_removed(n, read);
417480
wake(n);
418481
co_return true;
419482
}

src/v/cluster_link/schema_registry_sync/reconciler.h

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#pragma once
1313

14+
#include "cluster_link/model/types.h"
1415
#include "cluster_link/schema_registry_sync/scope.h"
1516
#include "cluster_link/schema_registry_sync/source_reader.h"
1617
#include "container/chunked_hash_map.h"
@@ -37,9 +38,13 @@ struct reconcile_stats {
3738
/// Source versions imported into the destination this run.
3839
uint64_t versions_changed{0};
3940
/// Per-item failures (unresolvable references, import conflicts, unparsable
40-
/// schemas). These do not abort the run; the reconciler counts them and
41-
/// continues.
41+
/// schemas, and -- under the FAIL policy -- source schemas carrying
42+
/// unsupported fields). These do not abort the run; the reconciler counts
43+
/// them and continues so the rest of the subjects still replicate.
4244
uint64_t errors{0};
45+
/// Unsupported fields removed from imported schemas this run under the
46+
/// REMOVE policy.
47+
uint64_t unsupported_features_removed{0};
4348
};
4449

4550
/// The set of changes a reconcile applies: each upsert is a (context, subject,
@@ -97,7 +102,9 @@ class reconciler {
97102
schema::registry* destination,
98103
ss::noncopyable_function<bool(const ppsr::context_subject&)> in_scope,
99104
const context_mapper& mapper,
100-
limits lim);
105+
limits lim,
106+
model::schema_registry_sync_config::unsupported_feature_policy
107+
feature_policy);
101108

102109
/// Imports `work_set.upserts` referent-first.
103110
///
@@ -163,11 +170,31 @@ class reconciler {
163170
/// Imports a fetched read, classifying conflicts as per-item failures.
164171
/// Returns true on a successful import. Carries the whole
165172
/// source_schema_read (not just the schema) so the unsupported-feature
166-
/// policy can act on `read.unsupported` at the single per-node import
167-
/// point.
173+
/// policy can act on `read.unsupported` at the authoritative per-node
174+
/// import point (FAIL rejection and REMOVE accounting both happen here).
168175
ss::future<bool>
169176
import_body(const ppsr::subject_version& n, ppsr::source_schema_read read);
170177

178+
/// Under the FAIL policy, treats a source schema carrying unsupported
179+
/// fields as a per-item failure: logs the offending fields, marks the node
180+
/// (and its dependents) failed via fail(n), and returns true so the caller
181+
/// skips the import. The rest of the sync proceeds -- fail-fast is reserved
182+
/// for global errors like source unavailability. Returns false (a no-op)
183+
/// under REMOVE or when `read` carries nothing unsupported.
184+
///
185+
/// Called by `import_body` (authoritative gate) and by `discover` before
186+
/// queuing a node's references.
187+
bool fail_if_contains_unsupported(
188+
const ppsr::subject_version& n, const ppsr::source_schema_read& read);
189+
190+
/// Under the REMOVE policy, accounts for the unsupported fields the parser
191+
/// already dropped from `read`: logs them and adds them to
192+
/// `unsupported_features_removed`. A no-op under FAIL or when nothing was
193+
/// dropped. Call only after the import succeeds, so a failed import does
194+
/// not report features as removed.
195+
void count_if_contains_unsupported_removed(
196+
const ppsr::subject_version& n, const ppsr::source_schema_read& read);
197+
171198
/// Marks a node replicated and releases dependents whose in-degree hits 0.
172199
void wake(const ppsr::subject_version& n);
173200

@@ -189,6 +216,8 @@ class reconciler {
189216
ss::noncopyable_function<bool(const ppsr::context_subject&)> _in_scope;
190217
const context_mapper* _mapper;
191218
limits _limits;
219+
model::schema_registry_sync_config::unsupported_feature_policy
220+
_feature_policy;
192221

193222
chunked_hash_set<ppsr::subject_version> _replicated;
194223
chunked_hash_map<ppsr::subject_version, node_data> _nodes;

src/v/cluster_link/schema_registry_sync/tests/mirroring_task_test.cc

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,63 @@ TEST_F(mirroring_task_test, full_sync_imports_and_reports) {
251251
EXPECT_LT(index_of(all, "a"), index_of(all, "b"));
252252
}
253253

254+
TEST_F(mirroring_task_test, remove_policy_strips_and_counts_unsupported) {
255+
auto a = ppsr::context_subject::unqualified("a");
256+
_source_state.add(a, 1);
257+
_source_state.set_unsupported(a, 1, {{.json_pointer = "/ruleSet"}});
258+
259+
auto metadata = get_default_metadata();
260+
metadata.configuration.schema_registry_sync_cfg.api_mode()->feature_policy
261+
= model::schema_registry_sync_config::unsupported_feature_policy::remove;
262+
263+
lead_schema_registry();
264+
fixture()->upsert_link(std::move(metadata)).get();
265+
266+
auto status = wait_for_sync_status([](const auto& s) {
267+
return s.last_full_sync.has_value()
268+
&& !s.current_sync.has_value();
269+
}).get();
270+
ASSERT_TRUE(status.has_value());
271+
// The supported projection is imported and the removed feature is counted.
272+
EXPECT_EQ(status->last_full_sync->subject_versions_changed, 1);
273+
EXPECT_EQ(status->last_full_sync->unsupported_features_removed, 1);
274+
EXPECT_EQ(status->totals_since_task_start.unsupported_features_removed, 1);
275+
EXPECT_GE(index_of(_registry.get_all(), "a"), 0);
276+
}
277+
278+
TEST_F(mirroring_task_test, fail_policy_counts_unsupported_and_syncs_rest) {
279+
// Under FAIL an unsupported feature is a counted per-item error, not a
280+
// whole-sync abort: the offending subject is skipped while the rest sync,
281+
// and the task stays active. Fail-fast is reserved for global errors like
282+
// source unavailability.
283+
auto a = ppsr::context_subject::unqualified("a"); // unsupported -> skipped
284+
auto b = ppsr::context_subject::unqualified("b"); // clean -> imported
285+
_source_state.add(a, 1);
286+
_source_state.add(b, 1);
287+
_source_state.set_unsupported(a, 1, {{.json_pointer = "/ruleSet"}});
288+
289+
auto metadata = get_default_metadata();
290+
metadata.configuration.schema_registry_sync_cfg.api_mode()->feature_policy
291+
= model::schema_registry_sync_config::unsupported_feature_policy::fail;
292+
293+
lead_schema_registry();
294+
fixture()->upsert_link(std::move(metadata)).get();
295+
296+
// The full sync completes (the task does not fault): the unsupported
297+
// subject is counted as an error and skipped, and the clean subject is
298+
// imported.
299+
auto status = wait_for_sync_status([](const auto& s) {
300+
return s.last_full_sync.has_value()
301+
&& !s.current_sync.has_value();
302+
}).get();
303+
ASSERT_TRUE(status.has_value());
304+
EXPECT_EQ(status->last_full_sync->errors, 1);
305+
EXPECT_EQ(status->last_full_sync->subject_versions_changed, 1);
306+
EXPECT_EQ(status->last_full_sync->unsupported_features_removed, 0);
307+
EXPECT_EQ(index_of(_registry.get_all(), "a"), -1);
308+
EXPECT_GE(index_of(_registry.get_all(), "b"), 0);
309+
}
310+
254311
TEST_F(mirroring_task_test, source_unavailable_then_recovers) {
255312
_source_state.add(ppsr::context_subject::unqualified("orders-value"), 1);
256313
_source_state.list_subjects_error = srs::source_error{

0 commit comments

Comments
 (0)