Skip to content

Commit 78789b8

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 3d18f85 commit 78789b8

7 files changed

Lines changed: 253 additions & 15 deletions

File tree

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: 52 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();
@@ -289,6 +294,9 @@ reconciler::discover(const ppsr::subject_version& n, ss::abort_source& as) {
289294
co_return source_result<void>{};
290295
}
291296

297+
if (fail_if_unsupported(n, fetched.value())) {
298+
co_return source_result<void>{};
299+
}
292300
adjust_units(_mem, units, body_size(fetched.value().schema));
293301

294302
auto refs = resolve_refs(fetched.value().schema);
@@ -369,14 +377,44 @@ reconciler::do_import(const ppsr::subject_version& n, ss::abort_source& as) {
369377
co_return source_result<void>{};
370378
}
371379

380+
if (fail_if_unsupported(n, fetched.value())) {
381+
co_return source_result<void>{};
382+
}
372383
adjust_units(_mem, units, body_size(fetched.value().schema));
373384

374385
co_await import_body(n, std::move(fetched.value()));
375386
co_return source_result<void>{};
376387
}
377388

389+
bool reconciler::fail_if_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+
378412
ss::future<bool> reconciler::import_body(
379413
const ppsr::subject_version& n, ppsr::source_schema_read read) {
414+
const bool removing = _feature_policy
415+
== model::schema_registry_sync_config::
416+
unsupported_feature_policy::remove
417+
&& !read.unsupported.empty();
380418
data(n).state = node_state::importing;
381419
// The graph key `n` stays in the source namespace; only the schema written
382420
// to the destination is remapped.
@@ -414,6 +452,19 @@ ss::future<bool> reconciler::import_body(
414452
}
415453
data(n).state = node_state::done;
416454
++_stats->versions_changed;
455+
// Count and log the stripped features only after the projection actually
456+
// lands on the destination, so a failed import does not report features as
457+
// removed. read.schema is moved-from above, but read.unsupported is intact.
458+
if (removing) {
459+
vlog(
460+
cllog.info,
461+
"Removed {} unsupported schema feature(s) from {}/{}: {}",
462+
read.unsupported.size(),
463+
n.sub,
464+
n.version,
465+
fmt::join(read.unsupported, ", "));
466+
_stats->unsupported_features_removed += read.unsupported.size();
467+
}
417468
wake(n);
418469
co_return true;
419470
}

src/v/cluster_link/schema_registry_sync/reconciler.h

Lines changed: 21 additions & 3 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
///
@@ -168,6 +175,15 @@ class reconciler {
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+
bool fail_if_unsupported(
185+
const ppsr::subject_version& n, const ppsr::source_schema_read& read);
186+
171187
/// Marks a node replicated and releases dependents whose in-degree hits 0.
172188
void wake(const ppsr::subject_version& n);
173189

@@ -189,6 +205,8 @@ class reconciler {
189205
ss::noncopyable_function<bool(const ppsr::context_subject&)> _in_scope;
190206
const context_mapper* _mapper;
191207
limits _limits;
208+
model::schema_registry_sync_config::unsupported_feature_policy
209+
_feature_policy;
192210

193211
chunked_hash_set<ppsr::subject_version> _replicated;
194212
chunked_hash_map<ppsr::subject_version, node_data> _nodes;

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

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

0 commit comments

Comments
 (0)