[fix](jdbc) Add shared JDBC driver checksum loader#63676
Open
xylaaaaa wants to merge 2 commits into
Open
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for selectively bootstrapping Hive2/Hive3 datasets in the thirdparties Docker environment, and introduces JDBC driver checksum validation + driver registration helpers (with unit tests).
Changes:
- Add Hive “bootstrap groups” helper + group list files, and wire group selection into hive data extraction/download, HDFS copy, and HQL/run.sh execution.
- Add
JdbcDriverLoader(FE) andJdbcDriverUtils(java-common) to validate driver checksums before registering JDBC drivers, with accompanying tests. - Add a small pipeline helper script to output bootstrap group strings from a simple CLI input.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| regression-test/pipeline/common/get-hive-bootstrap-groups.sh | New CLI helper to emit hive bootstrap group strings. |
| fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcDriverLoader.java | New FE-side driver checksum validation + dynamic driver registration w/ caching. |
| fe/fe-core/src/test/java/org/apache/doris/catalog/JdbcDriverLoaderTest.java | New tests for checksum validation and registration flow. |
| fe/be-java-extensions/java-common/src/main/java/org/apache/doris/common/jdbc/JdbcDriverUtils.java | New BE/java-common checksum computation + driver registration utility. |
| fe/be-java-extensions/java-common/src/test/java/org/apache/doris/common/jdbc/JdbcDriverUtilsTest.java | New tests for checksum validation behavior. |
| docker/thirdparties/run-thirdparties-docker.sh | Plumbs bootstrap groups env into hive2/hive3 startup and data preparation. |
| docker/thirdparties/docker-compose/hive/scripts/prepare-hive-data.sh | Filters which archives/downloads run based on selected bootstrap groups. |
| docker/thirdparties/docker-compose/hive/scripts/hive-metastore.sh | Filters run.sh/HDFS copy/HQL creation steps based on selected bootstrap groups. |
| docker/thirdparties/docker-compose/hive/scripts/bootstrap/*.list | New mapping lists used to categorize items as hive2_only/hive3_only. |
| docker/thirdparties/docker-compose/hive/scripts/bootstrap/bootstrap-groups.sh | New shared bash library for group normalization/selection logic. |
| docker/thirdparties/docker-compose/hive/hadoop-hive.env.tpl | Passes HIVE_BOOTSTRAP_GROUPS into hive containers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cd - | ||
| else | ||
| echo "${CUR_DIR}/tpch1.db exist, continue !" | ||
| BOOTSTRAP_GROUPS="$(bootstrap_normalize_groups "${HIVE_BOOTSTRAP_GROUPS:-}")" |
| set -e -x | ||
|
|
||
| . /mnt/scripts/bootstrap/bootstrap-groups.sh | ||
| BOOTSTRAP_GROUPS="$(bootstrap_normalize_groups "${HIVE_BOOTSTRAP_GROUPS:-}")" |
| fi | ||
|
|
||
| if [[ -d "${local_path}" && -z "$(ls "${local_path}")" ]]; then | ||
| echo "${local_path} does not exist" |
Comment on lines
+65
to
+71
| validateDriverChecksum(driverUrl, expectedChecksum); | ||
| } catch (DdlException e) { | ||
| throw new IllegalArgumentException(e.getMessage(), e); | ||
| } | ||
|
|
||
| try { | ||
| String fullDriverUrl = JdbcResource.getFullDriverUrl(driverUrl); |
Comment on lines
+65
to
+71
| validateDriverChecksum(driverUrl, expectedChecksum); | ||
| } catch (DdlException e) { | ||
| throw new IllegalArgumentException(e.getMessage(), e); | ||
| } | ||
|
|
||
| try { | ||
| String fullDriverUrl = JdbcResource.getFullDriverUrl(driverUrl); |
| public class JdbcDriverUtils { | ||
| private static final Logger LOG = Logger.getLogger(JdbcDriverUtils.class); | ||
| private static final int HTTP_TIMEOUT_MS = 10000; | ||
| private static final Map<URL, ClassLoader> DRIVER_CLASS_LOADER_CACHE = new ConcurrentHashMap<>(); |
Comment on lines
+71
to
+72
| ClassLoader classLoader = DRIVER_CLASS_LOADER_CACHE.computeIfAbsent(url, u -> | ||
| URLClassLoader.newInstance(new URL[] {u}, parentClassLoader)); |
Comment on lines
+36
to
+76
| boolean oldRunningUnitTest = FeConstants.runningUnitTest; | ||
| FeConstants.runningUnitTest = false; | ||
| try { | ||
| String driverUrl = createDriverUrl(); | ||
|
|
||
| DdlException exception = Assert.assertThrows(DdlException.class, | ||
| () -> JdbcDriverLoader.validateDriverChecksum(driverUrl, "bad-checksum")); | ||
| Assert.assertTrue(exception.getMessage().contains("does not match the computed checksum")); | ||
| } finally { | ||
| FeConstants.runningUnitTest = oldRunningUnitTest; | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testRegisterDriverValidatesChecksumBeforeLoadingClass() throws Exception { | ||
| boolean oldRunningUnitTest = FeConstants.runningUnitTest; | ||
| FeConstants.runningUnitTest = false; | ||
| try { | ||
| String driverUrl = createDriverUrl(); | ||
|
|
||
| IllegalArgumentException exception = Assert.assertThrows(IllegalArgumentException.class, | ||
| () -> JdbcDriverLoader.registerDriver(driverUrl, | ||
| "org.apache.doris.catalog.NotExistingDriver", "bad-checksum", | ||
| getClass().getClassLoader())); | ||
| Assert.assertTrue(exception.getMessage().contains("does not match the computed checksum")); | ||
| } finally { | ||
| FeConstants.runningUnitTest = oldRunningUnitTest; | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testValidateDriverChecksumReturnsComputedChecksum() throws Exception { | ||
| boolean oldRunningUnitTest = FeConstants.runningUnitTest; | ||
| FeConstants.runningUnitTest = false; | ||
| try { | ||
| String driverUrl = createDriverUrl(); | ||
|
|
||
| Assert.assertEquals(DRIVER_CHECKSUM, JdbcDriverLoader.validateDriverChecksum(driverUrl, DRIVER_CHECKSUM)); | ||
| Assert.assertEquals(DRIVER_CHECKSUM, JdbcDriverLoader.validateDriverChecksum(driverUrl, "")); | ||
| } finally { | ||
| FeConstants.runningUnitTest = oldRunningUnitTest; |
Comment on lines
+82
to
+83
| Files.write(driverPath, DRIVER_BYTES.getBytes(StandardCharsets.UTF_8)); | ||
| return "file://" + driverPath.toAbsolutePath(); |
Comment on lines
+26
to
+38
| case "${1:-}" in | ||
| hive2) | ||
| echo "common,hive2_only" | ||
| ;; | ||
| hive3) | ||
| echo "common,hive3_only" | ||
| ;; | ||
| both) | ||
| echo "common,hive2_only,hive3_only" | ||
| ;; | ||
| all) | ||
| echo "all" | ||
| ;; |
xylaaaaa
force-pushed
the
codex/jdbc-driver-checksum-loader
branch
2 times, most recently
from
f77eb11 to
ae3fedb
Compare
### What problem does this PR solve? Issue Number: None Related PR: apache#61094 Problem Summary: Add shared FE and Java extension utilities to validate JDBC driver checksums before dynamically loading JDBC driver jars. ### Release note None ### Check List (For Author) - Test: Unit Test - MAVEN_OPTS='-Dmaven.build.cache.enabled=false' ./run-fe-ut.sh --run org.apache.doris.catalog.JdbcDriverLoaderTest,org.apache.doris.common.jdbc.JdbcDriverUtilsTest - Behavior changed: No - Does this need documentation: No
xylaaaaa
force-pushed
the
codex/jdbc-driver-checksum-loader
branch
from
ae3fedb to
408141c
Compare
Contributor
Author
|
run buildall |
1 similar comment
Contributor
Author
|
run buildall |
### What problem does this PR solve? Issue Number: None Related PR: apache#63676 Problem Summary: Reuse the shared JDBC driver classloader preparation for BE JDBC test connection and propagate driver checksum to the Java tester. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.common.jdbc.JdbcDriverUtilsTest - Behavior changed: Yes. BE JDBC test connection validates the driver checksum when checksum is provided. - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
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: None
Related PR: #61094
Problem Summary: Add shared FE and Java extension utilities to validate JDBC driver checksums before dynamically loading JDBC driver jars. This provides a common path for future JDBC-based connectors to verify driver jars before class loading.
Release note
None
Check List (For Author)