[test](catalog) verify 4.1.3 catalogs survive the connector-SPI cutover, with fixtures generated by real 4.1.3 bytecode - #66214
Merged
morningman merged 4 commits intoJul 29, 2026
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
morningman
force-pushed
the
branch-catalog-spi
branch
from
July 29, 2026 09:35
09f86f0 to
f672e78
Compare
morningman
requested review from
924060929,
Gabriel39,
dataroaring,
englefly,
gavinchou,
liaoxin01,
luwei16,
morrySnow,
mymeiyi,
seawinde and
starocean999
as code owners
July 29, 2026 09:35
morningman
force-pushed
the
branch-catalog-spi
branch
from
July 29, 2026 13:18
f672e78 to
0da96f1
Compare
…ing replay Replaying OP_CREATE_CATALOG builds the connector synchronously, on the thread that is starting the FE, and EditLog's fallback turns anything thrown there into System.exit(-1). So a connector that rejects a property an older FE stored without validating -- or a half-installed plugin throwing NoClassDefFoundError -- keeps the whole FE from starting instead of making one catalog unusable. The image path never had this problem because it builds the connector lazily. Catch on the replay arm only and fall through to the degraded registration that already exists for an unclaimed type; the failure resurfaces at first access as an ordinary query error. Interactive CREATE CATALOG still fails loud, because the user is waiting for the error. Catching RuntimeException | Error rather than Exception is deliberate: a half-installed plugin throws NoClassDefFoundError, not a RuntimeException. Covered by Legacy413JournalReplayTest#connectorConstructorThatThrowsMustNotTakeTheFeDown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mework
Adds golden FE metadata fixtures produced by real Doris 4.1.3 bytecode, and
the tests that replay them on this branch. Goal: a catalog created on 4.1.3
must migrate onto the connector SPI and remain usable, through both the image
and the edit-log path.
The fixtures are generated, never hand-written. 4.1.3's wire format has at
least five details a hand-authored blob gets wrong: HTML escaping is on
(jdbc_url persists '=' and '&' escaped), serializeNulls is off (the resource
key vanishes when null), taap is polymorphic ({} when empty, an array of
2-tuples when not), clazz is written first, and the create path rewrites the
property map before it is persisted. The generator, its provenance and the
regeneration recipe live next to the fixtures; output is byte-deterministic.
Fixture families, and why the resource-backed one is not optional: with a
"type" property present, deleting the migration entirely leaves every
assertion green, because getType() just reads what was already on the wire.
Only the resource-backed catalogs -- which persist no "type" and recover it
from logType -- can fail when the migration is removed. Mutation-verified.
Tests:
Legacy413ImageMigrationTest all 20 legacy catalog labels migrate;
engine-owned types (doris) do not; type
recovery incl. the trino-connector hyphen;
comment/lastUpdateTime/taap/jdbc_url fidelity
Legacy413LabelRegistryTest every label 4.1.3 could write still resolves,
compared registry-to-registry rather than
against a hand-maintained list
Legacy413JournalReplayTest edit-log replay, property fidelity through to
the connector, degraded paths
Legacy413CloudImageTest cloud mode; one guard, not a second matrix,
because the cloud and non-cloud modules differ
in exactly one entry
Legacy413ProviderTypeContractTest the recovered type must be a string a REAL
shipped provider answers to -- the two halves
are literals in different modules
fe-core gains test-scope-only dependencies on the connector modules, matching
the existing fe-filesystem-* arrangement, solely so that last test can compare
both halves. Production still loads connectors from plugins/connector/.
Anti-vacuity: five mutations were applied and each turned the predicted test
red -- disabling the type backfill, dropping the TRINO_CONNECTOR case,
deleting a GSON label, reverting the replay guard, and removing a connector
from the shipped set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
force-pushed
the
catalog-spi-413-upgrade-compat-ut
branch
from
July 29, 2026 13:22
790206f to
95b7435
Compare
…vn test` can compile against them fe-connector-hms-hive-shade and fe-connector-paimon-hive-shade carry no sources: their Hive metastore content exists only once maven-shade has run. With both maven-jar-plugin and maven-shade-plugin bound to `package`, any build that stops earlier leaves those modules with nothing but an (empty) target/classes, and that is what the reactor then hands to their consumers. run-fe-ut.sh builds with `mvn test`, which stops before `package`. Once fe-core took test-scope dependencies on the connector impl modules, the FE UT reactor grew from 40 to 59 modules and began pulling fe-connector-hms in, where every org.apache.hadoop.hive.* import failed to resolve (TeamCity FE UT 1009434: "package org.apache.hadoop.hive.metastore.api does not exist", build aborted before fe-core ran a single test). fe-connector-paimon would have failed the same way on HiveConf one module later. Move both executions to process-classes -- the last phase still ahead of test-compile -- keeping maven-jar-plugin declared first so the main artifact is present before shade, which is what the package-phase pin was protecting under a parallel reactor. Cost is ~21s per FE UT run (paimon 17s, hms 4s), since the build cache is disabled at project level for both modules. Verified: the full 59-module `mvn -pl fe-common,fe-core -am test-compile` succeeds and reproducibly fails on fe-connector-hms without this change; both shade jars rebuild identical to the previous ones (same byte size, same 7940 / 55313 entry listings); `package` still runs each shade exactly once and the hive / paimon / iceberg / hudi plugin zips each carry their shade jar. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019JZp6xJNzwavabuea7bsAP
…-classes) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019JZp6xJNzwavabuea7bsAP
28 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: #65185
Problem Summary:
Part of the catalog-SPI migration (#65185). Review round 23, targeting
branch-catalog-spi.This branch has moved every data source out of
fe-coreand behind the connector SPI. One question ithad never answered with evidence: a catalog created on 4.1.3 — is it still there, still typed
correctly and still usable after the FE binary is replaced with one of these? This round answers it
for both metadata paths (image load and edit-log replay), and fixes the two defects that answering it
exposed.
1. The fixtures are produced by real 4.1.3 bytecode. Nothing here is hand-written.
fe/fe-core/src/test/resources/upgrade/413/holds an FE imagedatasourcemodule (framed exactly asMetaWriterwrites it), its cloud-mode twin, 19 BDBJE journal entries inJournalEntitywire form,and 4.1.3's own three GSON label registries read reflectively out of
RuntimeTypeAdapterFactory. Allof it was generated by running a committed generator against the
4.1.3tag (annotated tag, commit7126cf65d96).PROVENANCE.txtrecords the source tag, the gson version, the sha256 of both binaryfixtures and the exact regeneration recipe; output is byte-deterministic across JVM runs.
A hand-authored blob only encodes what someone believed 4.1.3 writes, and 4.1.3's wire format has at
least five details that are easy to get wrong — each one observed in these bytes:
jdbc_urlpersists with=and&escaped.serializeNullsis off:catalogProperty.resourceis absent entirely when null, notnull.taapis polymorphic underenableComplexMapKeySerialization:{}when empty, an array of2-tuples when not.
clazzis written first, thenExternalCatalog's 8 fields in declaration order.setDefaultPropsIfMissinginjects properties andJDBC rewrites
jdbc_urlbefore it ever reaches the wire.2. One fixture family is the only one that can fail, and it is the one that nearly got cut
24 image entries: the internal catalog plus 23 externals, in three families.
CatalogFactory.createFromLog; props docarry
"type". This is what a catalog created on 4.1.3 looks like. 10015(
RemoteDorisExternalCatalog) is a control: the engine implementsdorisitself, so it mustnot be remapped onto
PluginDrivenExternalCatalog.catalogProperty.resourceis set and there is no"type"property, because in 4.1.3 the concrete class carried the type.
4.1.3's own factories can no longer produce (the paimon factory always returns the base class,
lakesoul creation is rejected outright). Carry-forward from ≤ 4.0 metadata.
G2 is load-bearing and was measured, not assumed: delete the type backfill in
PluginDrivenExternalCatalog.gsonPostProcessentirely and every G1 assertion stays green, becausegetType()just reads the property that was already on the wire. Only the resource-backed catalogs —which persist no
"type"and must recover it fromlogType— can go red. Within G2, 10023 is itselfload-bearing:
TRINO_CONNECTORis the onlylogTypewhosename().toLowerCase()(
trino_connector) differs from the type string the connector answers to (trino-connector), so abackfill that merely lowercases the enum passes the other three and silently kills every migrated
trino catalog.
3. What the tests pin — 5 classes, 21 cases
Legacy413ImageMigrationTestPluginDrivenExternalCatalog(assertSame, notinstanceof— a subclass remap is the same silent downgrade); engine-owned types and the internal catalog keep their own class; type recovery incl. the trino hyphen; identity / comment /lastUpdateTime/taap/jdbc_urlfidelityLegacy413LabelRegistryTestLegacy413JournalReplayTestOP_CREATE_CATALOG4.1.3 could write; the properties handed to the connector are exactly what was persisted; the two degraded pathsLegacy413CloudImageTestLegacy413ProviderTypeContractTestAnti-vacuity: five mutations were applied and each turned the predicted test red — disabling the
type backfill, dropping the
TRINO_CONNECTORcase, deleting a GSON label, reverting the replay guard,and removing a connector from the shipped set.
4. Defect found: a connector constructor could stop the FE from starting
Replaying
OP_CREATE_CATALOGbuilds the connector synchronously, on the thread that is starting theFE, and
EditLog's fallback turns anything thrown there intoSystem.exit(-1). So a connector thatrejects a property an older FE stored without validating — or a half-installed plugin throwing
NoClassDefFoundError— keeps the whole FE down instead of making one catalog unusable. The image pathnever had this problem: it builds the connector lazily.
CatalogFactorynow catches on the replay arm only and falls through to the degraded registrationthat already exists for an unclaimed type; the failure resurfaces at first access as an ordinary query
error. Interactive
CREATE CATALOGstill fails loud, because a user is waiting for the error. CatchingRuntimeException | Errorrather thanExceptionis deliberate — a half-installed plugin throwsNoClassDefFoundError, not aRuntimeException. Covered byLegacy413JournalReplayTest#connectorConstructorThatThrowsMustNotTakeTheFeDown.5. Defect found: FE UT could not compile its own reactor
fe-connector-hms-hive-shadeandfe-connector-paimon-hive-shadecarry no sources — their Hivemetastore content exists only once
maven-shadehas run. With bothmaven-jar-pluginandmaven-shade-pluginbound topackage, any build that stops earlier leaves those modules with nothingbut an empty
target/classes, and that is what the reactor hands their consumers.run-fe-ut.shbuilds withmvn test, which stops beforepackage. Oncefe-coretook the test-scopedependencies of item 6, the FE UT reactor grew from 40 to 59 modules and began pulling in
fe-connector-hms, where everyorg.apache.hadoop.hive.*import failed to resolve — the build abortedbefore
fe-coreran a single test.fe-connector-paimonwould have failed the same way onHiveConfone module later.
Both executions move to
process-classes, the last phase still ahead oftest-compile, keepingmaven-jar-plugindeclared first so the main artifact exists before shade (what thepackagepin wasprotecting under a parallel reactor). Cost is ~21s per FE UT run, since the build cache is disabled at
project level for both modules.
6.
fe-coretakes test-scope-only dependencies on the connector modulesSame arrangement as the 14 existing
fe-filesystem-*entries, and it exists for exactly one assertionthat cannot be made without it: the catalog type string an upgraded 4.1.3 catalog recovers must be a
string a real shipped provider answers to, and the two halves of that contract are string literals
in different modules. Rename
TrinoConnectorProvider.getType()to"trino"and every other test inthis repository stays green while every migrated trino catalog is permanently dead. Production is
unchanged: connectors are still loaded from
plugins/connector/, andfe-corehas no connectordependency at compile or runtime scope.
7. Out of scope, stated rather than implied
"clazz":"PluginDrivenExternalCatalog", which no 4.1.3 FE can parse, so once an upgraded FEcheckpoints, rolling the binary back to 4.1.3 cannot load its own image. One-way door, by decision.
property fidelity through to the connector. It does not prove a query against a real backend
succeeds. No offline replay of a production image and no binary-swap upgrade test were run.
Release note
None
Check List (For Author)
Local verification, at the coverage it actually has:
fe-corecases, all green locally.mvn -pl fe-common,fe-core -am test-compilereproducibly fails onfe-connector-hmswithout thechange and succeeds with it; both shade jars rebuild to the same byte size and the same 7940 / 55313
entry listings as before;
packagestill runs each shade exactly once and the hive / paimon /iceberg / hudi plugin zips each carry their shade jar.
would want one (an actual binary-swap upgrade) is explicitly out of scope, see item 7.
Known red on this head, both being fixed:
License Check — the four generated
.txtfixtures (PROVENANCE.txt,labels.{ds,db,tbl}.txt)carry no license header and cannot: every non-blank line is machine-read. They need
.licenserc.yamlentries, as
connector-metadata-methods.txtalready has.FE UT — the compile abort of item 5 is gone (fe-core tests now run), but 8
DorisHttpTestCasesubclasses fail with
Servlet class org.apache.jasper.servlet.JspServlet is not a jakarta.servlet.Servlet. Root cause is the test-scope dependency of item 6:fe-connector-paimon-hive-shadebundles a javax-eraorg.apache.jasper.servlet.JspServlet, soSpring Boot's Jetty factory detects "JSP support present" and registers it into a Jetty 12 / ee10
container that only accepts
jakarta.servlet.Servlet. It is classpath pollution from the shadewhitelist, not a product defect.
Behavior changed:
Edit-log replay of
OP_CREATE_CATALOGno longer takes the FE down when the connector constructorthrows. That catalog is registered in degraded mode instead and reports the failure at first access.
Interactive
CREATE CATALOGis unchanged and still fails loud.Nothing user-facing changes. The upgrade behaviour being pinned here is what an operator already
expects; the degraded-replay path is only reachable when a plugin is broken.
Check List (For Reviewer who merge this PR)