From e2649ef4e66a006d99b13b9e92bb1aa447bb82eb Mon Sep 17 00:00:00 2001 From: zy-kkk Date: Tue, 20 May 2025 10:20:08 +0800 Subject: [PATCH] [hotfix](jdbc catalog) Fix jdbcclient repeated initialization (#51036) ### What problem does this PR solve? Related PR: #50901 Problem Summary: In the previous PR, I added makeSureInitialized() to configureJdbcTable, which will create the jdbcclient if it does not exist. However, in the testJdbcConnection process, makeSureInitialized() is bypassed and initLocalObjectsImpl is used directly. This will cause a jdbcclient to be created again when configureJdbcTable is used in testJdbcConnection, and the previous jdbcclient is not closed, resulting in a connection leak. In the modification of this PR, I used a completely independent testClient for the TestConnection process to avoid this problem. --- .../datasource/jdbc/JdbcExternalCatalog.java | 50 +++++++++++-------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java index 34afa08ff8d773..b562a0329fb3dc 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java @@ -220,6 +220,10 @@ public boolean isTestConnection() { @Override protected void initLocalObjectsImpl() { + jdbcClient = createJdbcClient(); + } + + private JdbcClient createJdbcClient() { JdbcClientConfig jdbcClientConfig = new JdbcClientConfig() .setCatalog(this.name) .setUser(getJdbcUser()) @@ -236,7 +240,7 @@ protected void initLocalObjectsImpl() { .setConnectionPoolMaxWaitTime(getConnectionPoolMaxWaitTime()) .setConnectionPoolKeepAlive(isConnectionPoolKeepAlive()); - jdbcClient = JdbcClient.createJdbcClient(jdbcClientConfig); + return JdbcClient.createJdbcClient(jdbcClientConfig); } @Override @@ -337,9 +341,13 @@ public List getColumnsFromQuery(String query) { public void configureJdbcTable(JdbcTable jdbcTable, String tableName) { makeSureInitialized(); + setCommonJdbcTableProperties(jdbcTable, tableName, this.jdbcClient); + } + + private void setCommonJdbcTableProperties(JdbcTable jdbcTable, String tableName, JdbcClient jdbcClient) { jdbcTable.setCatalogId(this.getId()); jdbcTable.setExternalTableName(tableName); - jdbcTable.setJdbcTypeName(this.getDatabaseTypeName()); + jdbcTable.setJdbcTypeName(jdbcClient.getDbType()); jdbcTable.setJdbcUrl(this.getJdbcUrl()); jdbcTable.setJdbcUser(this.getJdbcUser()); jdbcTable.setJdbcPasswd(this.getJdbcPasswd()); @@ -360,22 +368,23 @@ private void testJdbcConnection() throws DdlException { return; } if (isTestConnection()) { + JdbcClient testClient = null; try { - initLocalObjectsImpl(); - testFeToJdbcConnection(); - testBeToJdbcConnection(); + testClient = createJdbcClient(); + testFeToJdbcConnection(testClient); + testBeToJdbcConnection(testClient); } finally { - if (jdbcClient != null) { - jdbcClient.closeClient(); - jdbcClient = null; + if (testClient != null) { + testClient.closeClient(); + testClient = null; } } } } - private void testFeToJdbcConnection() throws DdlException { + private void testFeToJdbcConnection(JdbcClient testClient) throws DdlException { try { - jdbcClient.testConnection(); + testClient.testConnection(); } catch (JdbcClientException e) { String errorMessage = "Test FE Connection to JDBC Failed: " + e.getMessage(); LOG.warn(errorMessage, e); @@ -383,7 +392,7 @@ private void testFeToJdbcConnection() throws DdlException { } } - private void testBeToJdbcConnection() throws DdlException { + private void testBeToJdbcConnection(JdbcClient testClient) throws DdlException { Backend aliveBe = null; for (Backend be : Env.getCurrentSystemInfo().getIdToBackend().values()) { if (be.isAlive()) { @@ -395,11 +404,11 @@ private void testBeToJdbcConnection() throws DdlException { } TNetworkAddress address = new TNetworkAddress(aliveBe.getHost(), aliveBe.getBrpcPort()); try { - JdbcTable jdbcTable = getTestConnectionJdbcTable(); + JdbcTable testTable = getTestConnectionJdbcTable(testClient); PJdbcTestConnectionRequest request = InternalService.PJdbcTestConnectionRequest.newBuilder() - .setJdbcTable(ByteString.copyFrom(new TSerializer().serialize(jdbcTable.toThrift()))) - .setJdbcTableType(jdbcTable.getJdbcTableType().getValue()) - .setQueryStr(jdbcClient.getTestQuery()).build(); + .setJdbcTable(ByteString.copyFrom(new TSerializer().serialize(testTable.toThrift()))) + .setJdbcTableType(testTable.getJdbcTableType().getValue()) + .setQueryStr(testClient.getTestQuery()).build(); InternalService.PJdbcTestConnectionResult result = null; Future future = BackendServiceProxy.getInstance() .testJdbcConnection(address, request); @@ -413,14 +422,13 @@ private void testBeToJdbcConnection() throws DdlException { } } - private JdbcTable getTestConnectionJdbcTable() throws DdlException { - JdbcTable jdbcTable = new JdbcTable(0, "test_jdbc_connection", Lists.newArrayList(), + private JdbcTable getTestConnectionJdbcTable(JdbcClient testClient) throws DdlException { + JdbcTable testTable = new JdbcTable(0, "test_jdbc_connection", Lists.newArrayList(), TableType.JDBC_EXTERNAL_TABLE); - this.configureJdbcTable(jdbcTable, "test_jdbc_connection"); - + setCommonJdbcTableProperties(testTable, "test_jdbc_connection", testClient); // Special checksum computation - jdbcTable.setCheckSum(JdbcResource.computeObjectChecksum(this.getDriverUrl())); + testTable.setCheckSum(JdbcResource.computeObjectChecksum(this.getDriverUrl())); - return jdbcTable; + return testTable; } }