Skip to content

[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 into
apache:branch-catalog-spifrom
morningman:catalog-spi-413-upgrade-compat-ut
Jul 29, 2026
Merged

[test](catalog) verify 4.1.3 catalogs survive the connector-SPI cutover, with fixtures generated by real 4.1.3 bytecode#66214
morningman merged 4 commits into
apache:branch-catalog-spifrom
morningman:catalog-spi-413-upgrade-compat-ut

Conversation

@morningman

@morningman morningman commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

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-core and behind the connector SPI. One question it
had 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 image datasource module (framed exactly as
MetaWriter writes it), its cloud-mode twin, 19 BDBJE journal entries in JournalEntity wire form,
and 4.1.3's own three GSON label registries read reflectively out of RuntimeTypeAdapterFactory. All
of it was generated by running a committed generator against the 4.1.3 tag (annotated tag, commit
7126cf65d96). PROVENANCE.txt records the source tag, the gson version, the sha256 of both binary
fixtures 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:

  1. HTML escaping is on: jdbc_url persists with = and & escaped.
  2. serializeNulls is off: catalogProperty.resource is absent entirely when null, not null.
  3. taap is polymorphic under enableComplexMapKeySerialization: {} when empty, an array of
    2-tuples when not.
  4. clazz is written first, then ExternalCatalog's 8 fields in declaration order.
  5. What is persisted is not what the user typed — setDefaultPropsIfMissing injects properties and
    JDBC rewrites jdbc_url before 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.

  • G1 (ids 10001–10015) — built through 4.1.3's own CatalogFactory.createFromLog; props do
    carry "type". This is what a catalog created on 4.1.3 looks like. 10015
    (RemoteDorisExternalCatalog) is a control: the engine implements doris itself, so it must
    not be remapped onto PluginDrivenExternalCatalog.
  • G2 (10020–10023) — resource-backed: catalogProperty.resource is set and there is no "type"
    property, because in 4.1.3 the concrete class carried the type.
  • G3 (10030–10034) — labels that exist in 4.1.3's registry and can appear in a 4.1 image but that
    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.gsonPostProcess entirely and every G1 assertion stays green
, because
getType() just reads the property that was already on the wire. Only the resource-backed catalogs —
which persist no "type" and must recover it from logType — can go red. Within G2, 10023 is itself
load-bearing: TRINO_CONNECTOR is the only logType whose name().toLowerCase()
(trino_connector) differs from the type string the connector answers to (trino-connector), so a
backfill 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

Test Pins
Legacy413ImageMigrationTest all 22 migrating catalogs load as PluginDrivenExternalCatalog (assertSame, not instanceof — 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_url fidelity
Legacy413LabelRegistryTest every catalog / database / table label 4.1.3 could emit still resolves — compared registry-to-registry against 4.1.3's real registries, not against a hand-maintained list; abstract-only labels recorded as unreachable
Legacy413JournalReplayTest edit-log replay of every OP_CREATE_CATALOG 4.1.3 could write; the properties handed to the connector are exactly what was persisted; the two degraded paths
Legacy413CloudImageTest cloud mode — one guard, not a second matrix, because the cloud and non-cloud image modules differ in exactly one entry (id 0), which is itself asserted
Legacy413ProviderTypeContractTest the recovered type string is one a really shipped provider answers to; sibling-only connectors never become a catalog type; engine-owned types are claimed by no plugin

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.

4. Defect found: a connector constructor could stop the FE from starting

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 down instead of making one catalog unusable. The image path
never had this problem: it builds the connector lazily.

CatalogFactory now catches on the replay arm only and falls 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 a 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.

5. Defect found: FE UT could not compile its own reactor

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 hands their consumers.

run-fe-ut.sh builds with mvn test, which stops before package. Once fe-core took the test-scope
dependencies of item 6, the FE UT reactor grew from 40 to 59 modules and began pulling in
fe-connector-hms, where every org.apache.hadoop.hive.* import failed to resolve — the build aborted
before fe-core ran a single test. fe-connector-paimon would have failed the same way on HiveConf
one module later.

Both executions move to process-classes, the last phase still ahead of test-compile, keeping
maven-jar-plugin declared first so the main artifact exists before shade (what the package pin was
protecting 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-core takes test-scope-only dependencies on the connector modules

Same arrangement as the 14 existing fe-filesystem-* entries, and it exists for exactly one assertion
that 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 in
this repository stays green while every migrated trino catalog is permanently dead. Production is
unchanged: connectors are still loaded from plugins/connector/, and fe-core has no connector
dependency at compile or runtime scope.

7. Out of scope, stated rather than implied

  • Downgrade is impossible, not merely untested. The branch writes
    "clazz":"PluginDrivenExternalCatalog", which no 4.1.3 FE can parse, so once an upgraded FE
    checkpoints, rolling the binary back to 4.1.3 cannot load its own image. One-way door, by decision.
  • This is FE UT, not a real upgrade. It proves routing to the right provider and item-by-item
    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)

  • Test
    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

Local verification, at the coverage it actually has:

  • 21 new fe-core cases, all green locally.
  • five mutations, each turning only the predicted test red (listed in item 3).
  • the fixture generator was re-run twice: identical sha256 both times.
  • the shade phase move was checked against the failure it fixes: the full 59-module
    mvn -pl fe-common,fe-core -am test-compile reproducibly fails on fe-connector-hms without the
    change and succeeds with it; both shade jars rebuild to the same byte size and the same 7940 / 55313
    entry listings as before; package still runs each shade exactly once and the hive / paimon /
    iceberg / hudi plugin zips each carry their shade jar.
  • no e2e / regression case — this PR adds none. Nothing here needs a cluster, and the thing that
    would want one (an actual binary-swap upgrade) is explicitly out of scope, see item 7.
  • no BE file is touched, so BE was not rebuilt.

Known red on this head, both being fixed:

  • License Check — the four generated .txt fixtures (PROVENANCE.txt, labels.{ds,db,tbl}.txt)
    carry no license header and cannot: every non-blank line is machine-read. They need .licenserc.yaml
    entries, as connector-metadata-methods.txt already has.

  • FE UT — the compile abort of item 5 is gone (fe-core tests now run), but 8 DorisHttpTestCase
    subclasses 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-shade bundles a javax-era org.apache.jasper.servlet.JspServlet, so
    Spring 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 shade
    whitelist, not a product defect.

  • Behavior changed:

    • No.
    • Yes.

Edit-log replay of OP_CREATE_CATALOG no longer takes the FE down when the connector constructor
throws. That catalog is registered in degraded mode instead and reports the failure at first access.
Interactive CREATE CATALOG is unchanged and still fails loud.

  • Does this need documentation?
    • No.
    • Yes.

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)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@morningman
morningman requested a review from CalvinKirs as a code owner July 29, 2026 06:06
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

morningman and others added 2 commits July 29, 2026 21:22
…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
morningman force-pushed the catalog-spi-413-upgrade-compat-ut branch from 790206f to 95b7435 Compare July 29, 2026 13:22
morningman and others added 2 commits July 30, 2026 00:50
…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
@morningman morningman changed the title [refactor] Catalog spi 413 upgrade compat ut [test](catalog) verify 4.1.3 catalogs survive the connector-SPI cutover, with fixtures generated by real 4.1.3 bytecode Jul 29, 2026
@morningman
morningman merged commit 0796e73 into apache:branch-catalog-spi Jul 29, 2026
40 of 46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants