Skip to content

Commit 4008fa6

Browse files
xunyin8claude
andcommitted
[controller] Preserve original strict failure during migration coercion
When the initial strict parse fails inside normalizeSchemaForMigration, log strictFailure at INFO before attempting the LOOSE_NUMERICS-based coercion, and on the post-coercion strict re-check attach strictFailure as a suppressed exception of whatever the second parse throws. For non-numeric violations (union default not first branch, bad names, dangling content) the post-coercion strict parse still fails — and without chaining, the operator only sees that second exception and has no idea what was wrong with the source schema. The suppressed entry puts the original message into the same stack trace. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 212f1ae commit 4008fa6

2 files changed

Lines changed: 24 additions & 4 deletions

File tree

services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2296,10 +2296,21 @@ String normalizeSchemaForMigration(String clusterName, String storeName, String
22962296
AvroSchemaParseUtils.parseSchemaFromJSONStrictValidation(schemaStr);
22972297
return schemaStr;
22982298
} catch (Exception strictFailure) {
2299+
LOGGER.info(
2300+
"Strict parse failed for store {} migrating into cluster {}; attempting numeric-default coercion.",
2301+
storeName,
2302+
clusterName,
2303+
strictFailure);
22992304
String coerced = AvroSchemaParseUtils.coerceNumericDefaultsToFieldType(schemaStr);
23002305
// Defensive: anything LOOSE_NUMERICS would have been lenient about (union default not first
2301-
// branch, bad names, etc.) is outside the coercion scope and must still fail strict.
2302-
AvroSchemaParseUtils.parseSchemaFromJSONStrictValidation(coerced);
2306+
// branch, bad names, etc.) is outside the coercion scope and must still fail strict. When it
2307+
// does, surface the *original* strict failure too — it's the one the operator needs to see.
2308+
try {
2309+
AvroSchemaParseUtils.parseSchemaFromJSONStrictValidation(coerced);
2310+
} catch (Exception coercedFailure) {
2311+
coercedFailure.addSuppressed(strictFailure);
2312+
throw coercedFailure;
2313+
}
23032314
if (!coerced.equals(schemaStr)) {
23042315
LOGGER.info(
23052316
"Coerced numeric default(s) in value schema for store {} migrating into cluster {}.",

services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdminWithoutCluster.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ public void testNormalizeReserializesLegacyNumericDefault() {
555555
com.linkedin.venice.schema.AvroSchemaParseUtils.parseSchemaFromJSONStrictValidation(normalized);
556556
}
557557

558-
@Test(expectedExceptions = Exception.class)
558+
@Test
559559
public void testNormalizeRejectsNonNumericStrictViolation() {
560560
String cluster = "venice-dest";
561561
String store = "migrating_store";
@@ -564,6 +564,15 @@ public void testNormalizeRejectsNonNumericStrictViolation() {
564564

565565
VeniceHelixAdmin admin = newNormalizeMock(cluster, store, cfg);
566566
// Migration context, but the violation is outside the numeric-default tier — must propagate.
567-
admin.normalizeSchemaForMigration(cluster, store, NON_NUMERIC_STRICT_VIOLATION_SCHEMA);
567+
// The original strict failure must ride along as a suppressed exception so the operator can
568+
// see what was actually wrong with the source schema, not just that post-coercion strict tripped.
569+
try {
570+
admin.normalizeSchemaForMigration(cluster, store, NON_NUMERIC_STRICT_VIOLATION_SCHEMA);
571+
Assert.fail("Expected normalize to throw on non-numeric strict violation");
572+
} catch (Exception coercedFailure) {
573+
Assert.assertTrue(
574+
coercedFailure.getSuppressed().length >= 1,
575+
"Original strict failure should be attached as a suppressed exception, but none was found");
576+
}
568577
}
569578
}

0 commit comments

Comments
 (0)