Skip to content

Commit

Permalink
[BugFix] shorten jdbc driver name to avoid file name too long (StarRo…
Browse files Browse the repository at this point in the history 
…cks#43277)

Signed-off-by: yanz <[email protected]>
  • Loading branch information
@dirtysalt
dirtysalt authored Mar 29, 2024
1 parent aeb613d commit 61ae36f
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 47 deletions.
9 changes: 5 additions & 4 deletions be/src/util/download_util.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ namespace starrocks {
Status DownloadUtil::download(const std::string& url, const std::string& target_file,
const std::string& expected_checksum) {
auto success = false;
auto tmp_file =
fmt::format("{}_{}_{}", target_file, expected_checksum, ThreadLocalUUIDGenerator::next_uuid_string());
auto tmp_file = fmt::format("{}_{}", target_file, ThreadLocalUUIDGenerator::next_uuid_string());
auto fp = fopen(tmp_file.c_str(), "w");
DeferOp defer([&]() {
if (fp != nullptr) {
Expand All @@ -42,8 +41,10 @@ Status DownloadUtil::download(const std::string& url, const std::string& target_
});

if (fp == nullptr) {
LOG(ERROR) << fmt::format("fail to open file {}", tmp_file);
return Status::InternalError(fmt::format("fail to open tmp file when downloading file from {}", url));
std::string errmsg = strerror(errno);
LOG(ERROR) << fmt::format("fail to open file. file = {}, error = {}", tmp_file, errmsg);
return Status::InternalError(
fmt::format("fail to open tmp file when downloading file from {}. error = {}", url, errmsg));
}

Md5Digest digest;
Expand Down
18 changes: 18 additions & 0 deletions be/test/util/download_util_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,22 @@ TEST_F(DownloadUtilTest, test_wrong_md5) {
ASSERT_EQ(1, files.size());
}

TEST_F(DownloadUtilTest, test_long_filename) {
const std::string target_file =
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
const std::string url = "http://127.0.0.1/a.txt";
const std::string checksum = "aaa";
Status st = DownloadUtil::download(url, target_file, checksum);
ASSERT_FALSE(st.ok()) << st;
}

} // namespace starrocks
28 changes: 25 additions & 3 deletions fe/fe-core/src/main/java/com/starrocks/catalog/JDBCTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import java.io.DataOutput;
import java.io.IOException;
import java.net.URI;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -180,9 +182,29 @@ private static String buildCatalogDriveName(String uri) {
// -> jdbc_postgresql_172.26.194.237_5432_db_pg_select
// requirement: it should be used as local path.
// and there is no ':' in it to avoid be parsed into non-local filesystem.
return uri.replace("//", "")
.replace("/", "_")
.replace(":", "_");
String ans = uri.replaceAll("[^0-9a-zA-Z]", "_");

// currently we use this uri as part of name of download file.
// so if this uri is too long, we might fail to write file on BE side.
// so here we have to shorten it to reduce fail probability because of long file name.

final String prefix = "jdbc_";
try {
// 256bits = 32bytes = 64hex chars.
MessageDigest digest = MessageDigest.getInstance("SHA-256");
digest.update(ans.getBytes());
byte[] hashBytes = digest.digest();
StringBuilder sb = new StringBuilder();
// it's for be side parsing: expect a _ in name.
sb.append(prefix);
for (byte b : hashBytes) {
sb.append(String.format("%02x", b));
}
ans = sb.toString();
} catch (NoSuchAlgorithmException e) {
// don't update `ans`.
}
return ans;
}

@Override
Expand Down
67 changes: 64 additions & 3 deletions fe/fe-core/src/test/java/com/starrocks/catalog/JDBCTableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.


package com.starrocks.catalog;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.starrocks.common.DdlException;
Expand All @@ -29,6 +29,7 @@
import org.junit.Before;
import org.junit.Test;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -137,7 +138,7 @@ public void testToThrift(@Mocked GlobalStateMgr globalStateMgr,

@Test
public void testToThriftWithoutResource(@Mocked GlobalStateMgr globalStateMgr,
@Mocked ResourceMgr resourceMgr) throws Exception {
@Mocked ResourceMgr resourceMgr) throws Exception {
String uri = "jdbc:mysql://127.0.0.1:3306";
Map<String, String> jdbcProperties = getMockedJDBCProperties(uri);
JDBCTable table = new JDBCTable(1000, "jdbc_table", columns, "db0", "catalog0", jdbcProperties);
Expand All @@ -153,7 +154,7 @@ public void testToThriftWithoutResource(@Mocked GlobalStateMgr globalStateMgr,

@Test
public void testToThriftWithJdbcParam(@Mocked GlobalStateMgr globalStateMgr,
@Mocked ResourceMgr resourceMgr) throws Exception {
@Mocked ResourceMgr resourceMgr) throws Exception {
String uri = "jdbc:mysql://127.0.0.1:3306?key=value";
Map<String, String> jdbcProperties = getMockedJDBCProperties(uri);
JDBCTable table = new JDBCTable(1000, "jdbc_table", columns, "db0", "catalog0", jdbcProperties);
Expand Down Expand Up @@ -218,4 +219,64 @@ public void testNoTable() throws Exception {
new JDBCTable(1000, "jdbc_table", columns, properties);
Assert.fail("No exception throws.");
}

@Test
public void testJDBCDriverName() {
try {
Map<String, String> properties = ImmutableMap.of(
"driver_class", "org.postgresql.Driver",
"checksum", "bef0b2e1c6edcd8647c24bed31e1a4ac",
"driver_url",
"http://x.com/postgresql-42.3.3.jar",
"type", "jdbc",
"user", "postgres",
"password", "postgres",
"jdbc_uri", "jdbc:postgresql://172.26.194.237:5432/db_pg_select"
);
List<Column> schema = new ArrayList<>();
schema.add(new Column("id", Type.INT));
JDBCTable jdbcTable = new JDBCTable(10, "tbl", schema, "db", "jdbc_catalog", properties);
TTableDescriptor tableDescriptor = jdbcTable.toThrift(null);
TJDBCTable table = tableDescriptor.getJdbcTable();
Assert.assertEquals(table.getJdbc_driver_name(),
"jdbc_f2ef8bf476c54395197451dd655c89dd6041f3d0dd9b906dc38518524af1ec64");
Assert.assertEquals(table.getJdbc_driver_url(), "http://x.com/postgresql-42.3.3.jar");
Assert.assertEquals(table.getJdbc_driver_checksum(), "bef0b2e1c6edcd8647c24bed31e1a4ac");
Assert.assertEquals(table.getJdbc_driver_class(), "org.postgresql.Driver");
} catch (Exception e) {
System.out.println(e.getMessage());
Assert.fail();
}
}

@Test
public void testJDBCDriverNameLong() {
try {
Map<String, String> properties = ImmutableMap.of(
"driver_class", "org.postgresql.Driver",
"checksum", "bef0b2e1c6edcd8647c24bed31e1a4ac",
"driver_url",
"http://x.com/postgresql-42.3.3.jar",
"type", "jdbc",
"user", "postgres",
"password", "postgres",
"jdbc_uri",
"jdbc:postgresql" +
"://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com/db_pg_select"
);
List<Column> schema = new ArrayList<>();
schema.add(new Column("id", Type.INT));
JDBCTable jdbcTable = new JDBCTable(10, "tbl", schema, "db", "jdbc_catalog", properties);
TTableDescriptor tableDescriptor = jdbcTable.toThrift(null);
TJDBCTable table = tableDescriptor.getJdbcTable();
Assert.assertEquals(table.getJdbc_driver_name(),
"jdbc_90377bb27298feecdca1ef8b2e9c2e00f0b012eabb9ad43437542d2e29ef52fc");
Assert.assertEquals(table.getJdbc_driver_url(), "http://x.com/postgresql-42.3.3.jar");
Assert.assertEquals(table.getJdbc_driver_checksum(), "bef0b2e1c6edcd8647c24bed31e1a4ac");
Assert.assertEquals(table.getJdbc_driver_class(), "org.postgresql.Driver");
} catch (Exception e) {
System.out.println(e.getMessage());
Assert.fail();
}
}
}
37 changes: 0 additions & 37 deletions fe/fe-core/src/test/java/com/starrocks/connector/jdbc/JDBCTableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,9 @@

package com.starrocks.connector.jdbc;

import com.google.common.collect.ImmutableMap;
import com.starrocks.catalog.Column;
import com.starrocks.catalog.JDBCTable;
import com.starrocks.catalog.Type;
import com.starrocks.thrift.TJDBCTable;
import com.starrocks.thrift.TTableDescriptor;
import org.junit.Assert;
import org.junit.Test;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;

public class JDBCTableTest {
Expand Down Expand Up @@ -57,32 +48,4 @@ public void testJDBCPartitionClass() {
Assert.fail();
}
}

@Test
public void testJDBCDriverName() {
try {
Map<String, String> properties = ImmutableMap.of(
"driver_class", "org.postgresql.Driver",
"checksum", "bef0b2e1c6edcd8647c24bed31e1a4ac",
"driver_url",
"http://x.com/postgresql-42.3.3.jar",
"type", "jdbc",
"user", "postgres",
"password", "postgres",
"jdbc_uri", "jdbc:postgresql://172.26.194.237:5432/db_pg_select"
);
List<Column> schema = new ArrayList<>();
schema.add(new Column("id", Type.INT));
JDBCTable jdbcTable = new JDBCTable(10, "tbl", schema, "db", "jdbc_catalog", properties);
TTableDescriptor tableDescriptor = jdbcTable.toThrift(null);
TJDBCTable table = tableDescriptor.getJdbcTable();
Assert.assertEquals(table.getJdbc_driver_name(), "jdbc_postgresql_172.26.194.237_5432_db_pg_select");
Assert.assertEquals(table.getJdbc_driver_url(), "http://x.com/postgresql-42.3.3.jar");
Assert.assertEquals(table.getJdbc_driver_checksum(), "bef0b2e1c6edcd8647c24bed31e1a4ac");
Assert.assertEquals(table.getJdbc_driver_class(), "org.postgresql.Driver");
} catch (Exception e) {
System.out.println(e.getMessage());
Assert.fail();
}
}
}

0 comments on commit 61ae36f

Please sign in to comment.