From 53d59a6b0add806e7c306b976259c355e07f1737 Mon Sep 17 00:00:00 2001 From: mchades Date: Fri, 17 May 2024 11:15:41 +0800 Subject: [PATCH] [#3249] improvement(kafka-catalog): clarify error message and shorten timeout (#3417) ### What changes were proposed in this pull request? - Transmit the real reason for the error to the user - shorten timeout from 60s to 15s - add thread info to the log - add default timeout config to the conf file ### Why are the changes needed? Fix: #3249 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? test added --- api/src/test/resources/log4j2.properties | 2 +- .../src/test/resources/log4j2.properties | 2 +- .../src/test/resources/log4j2.properties | 2 +- .../src/test/resources/log4j2.properties | 2 +- .../src/test/resources/log4j2.properties | 4 +-- .../src/test/resources/log4j2.properties | 2 +- .../catalog/kafka/KafkaCatalogOperations.java | 8 ++++- .../src/main/resources/kafka.conf | 4 ++- .../integration/test/CatalogKafkaIT.java | 33 ++++++++++++++++++- .../src/test/resources/log4j2.properties | 2 +- .../src/test/resources/log4j2.properties | 2 +- .../src/test/java/resources/log4j2.properties | 2 +- .../src/test/resources/log4j2.properties | 2 +- .../src/test/java/resources/log4j2.properties | 2 +- core/src/test/resources/log4j2.properties | 2 +- dev/docker/trino/conf/log4j2.properties | 4 +-- .../src/test/resources/log4j2.properties | 2 +- server/src/test/resources/log4j2.properties | 2 +- .../src/test/resources/log4j2.properties | 4 +-- 19 files changed, 61 insertions(+), 22 deletions(-) diff --git a/api/src/test/resources/log4j2.properties b/api/src/test/resources/log4j2.properties index 6beee06bcdc..b9e706603a0 100644 --- a/api/src/test/resources/log4j2.properties +++ b/api/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfigDemo appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Root logger level rootLogger.level = debug diff --git a/catalogs/catalog-hadoop/src/test/resources/log4j2.properties b/catalogs/catalog-hadoop/src/test/resources/log4j2.properties index 5c87cb57820..8438fbd3d33 100644 --- a/catalogs/catalog-hadoop/src/test/resources/log4j2.properties +++ b/catalogs/catalog-hadoop/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfig appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Log files location property.logPath = ${sys:gravitino.log.path:-catalog-hadoop/build/integration-test.log} diff --git a/catalogs/catalog-hive/src/test/resources/log4j2.properties b/catalogs/catalog-hive/src/test/resources/log4j2.properties index 47924627b40..ab88dcde35d 100644 --- a/catalogs/catalog-hive/src/test/resources/log4j2.properties +++ b/catalogs/catalog-hive/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfig appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Log files location property.logPath = ${sys:gravitino.log.path:-catalog-hive/build/hive-integration-test.log} diff --git a/catalogs/catalog-jdbc-doris/src/test/resources/log4j2.properties b/catalogs/catalog-jdbc-doris/src/test/resources/log4j2.properties index 9be08150eb0..c8307ac956f 100644 --- a/catalogs/catalog-jdbc-doris/src/test/resources/log4j2.properties +++ b/catalogs/catalog-jdbc-doris/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfig appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Log files location property.logPath = ${sys:gravitino.log.path:-catalog-jdbc-doris/build/doris-integration-test.log} diff --git a/catalogs/catalog-jdbc-mysql/src/test/resources/log4j2.properties b/catalogs/catalog-jdbc-mysql/src/test/resources/log4j2.properties index 476103ee781..9e5294bd1c6 100644 --- a/catalogs/catalog-jdbc-mysql/src/test/resources/log4j2.properties +++ b/catalogs/catalog-jdbc-mysql/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfig appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Log files location property.logPath = ${sys:gravitino.log.path:-catalog-jdbc-mysql/build/mysql-integration-test.log} @@ -23,7 +23,7 @@ appender.file.type = File appender.file.name = fileLogger appender.file.fileName = ${logPath} appender.file.layout.type = PatternLayout -appender.file.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.file.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Root logger level rootLogger.level = info diff --git a/catalogs/catalog-jdbc-postgresql/src/test/resources/log4j2.properties b/catalogs/catalog-jdbc-postgresql/src/test/resources/log4j2.properties index 33230753296..4b0ce16abb3 100644 --- a/catalogs/catalog-jdbc-postgresql/src/test/resources/log4j2.properties +++ b/catalogs/catalog-jdbc-postgresql/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfig appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Log files location property.logPath = ${sys:gravitino.log.path:-catalog-jdbc-postgresql/build/postgresql-integration-test.log} diff --git a/catalogs/catalog-kafka/src/main/java/com/datastrato/gravitino/catalog/kafka/KafkaCatalogOperations.java b/catalogs/catalog-kafka/src/main/java/com/datastrato/gravitino/catalog/kafka/KafkaCatalogOperations.java index 63261d176fe..bb5f60960e5 100644 --- a/catalogs/catalog-kafka/src/main/java/com/datastrato/gravitino/catalog/kafka/KafkaCatalogOperations.java +++ b/catalogs/catalog-kafka/src/main/java/com/datastrato/gravitino/catalog/kafka/KafkaCatalogOperations.java @@ -157,7 +157,13 @@ public NameIdentifier[] listTopics(Namespace namespace) throws NoSuchSchemaExcep return topicNames.stream() .map(name -> NameIdentifier.of(namespace, name)) .toArray(NameIdentifier[]::new); - } catch (InterruptedException | ExecutionException e) { + } catch (ExecutionException e) { + throw new RuntimeException( + String.format( + "Failed to list topics under the schema %s: %s", + namespace, e.getCause().getMessage()), + e); + } catch (InterruptedException e) { throw new RuntimeException("Failed to list topics under the schema " + namespace, e); } } diff --git a/catalogs/catalog-kafka/src/main/resources/kafka.conf b/catalogs/catalog-kafka/src/main/resources/kafka.conf index c7a58513c37..2e2467154d6 100644 --- a/catalogs/catalog-kafka/src/main/resources/kafka.conf +++ b/catalogs/catalog-kafka/src/main/resources/kafka.conf @@ -3,4 +3,6 @@ # This software is licensed under the Apache License version 2. # -# bootstrap.servers = localhost:9092 \ No newline at end of file +# bootstrap.servers = localhost:9092 +gravitino.bypass.request.timeout.ms = 15000 +gravitino.bypass.default.api.timeout.ms = 15000 \ No newline at end of file diff --git a/catalogs/catalog-kafka/src/test/java/com/datastrato/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java b/catalogs/catalog-kafka/src/test/java/com/datastrato/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java index 2636f2c2368..933cbe44100 100644 --- a/catalogs/catalog-kafka/src/test/java/com/datastrato/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java +++ b/catalogs/catalog-kafka/src/test/java/com/datastrato/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java @@ -7,6 +7,7 @@ import static com.datastrato.gravitino.catalog.kafka.KafkaCatalogPropertiesMetadata.BOOTSTRAP_SERVERS; import static com.datastrato.gravitino.catalog.kafka.KafkaTopicPropertiesMetadata.PARTITION_COUNT; import static com.datastrato.gravitino.catalog.kafka.KafkaTopicPropertiesMetadata.REPLICATION_FACTOR; +import static com.datastrato.gravitino.connector.BaseCatalog.CATALOG_BYPASS_PREFIX; import static com.datastrato.gravitino.integration.test.container.KafkaContainer.DEFAULT_BROKER_PORT; import com.datastrato.gravitino.Catalog; @@ -31,6 +32,7 @@ import java.util.Map; import java.util.concurrent.ExecutionException; import org.apache.kafka.clients.admin.AdminClient; +import org.apache.kafka.clients.admin.AdminClientConfig; import org.apache.kafka.clients.admin.Config; import org.apache.kafka.clients.admin.NewTopic; import org.apache.kafka.clients.admin.TopicDescription; @@ -119,7 +121,8 @@ public void testCatalog() throws ExecutionException, InterruptedException { Catalog createdCatalog = createCatalog(catalogName, comment, properties); Assertions.assertEquals(catalogName, createdCatalog.name()); Assertions.assertEquals(comment, createdCatalog.comment()); - Assertions.assertEquals(properties, createdCatalog.properties()); + Assertions.assertEquals( + kafkaBootstrapServers, createdCatalog.properties().get(BOOTSTRAP_SERVERS)); // test load catalog Catalog loadedCatalog = metalake.loadCatalog(NameIdentifier.of(METALAKE_NAME, catalogName)); @@ -173,6 +176,34 @@ public void testCatalogException() { ImmutableMap.of("abc", "2"))); Assertions.assertTrue( exception.getMessage().contains("Missing configuration: bootstrap.servers")); + + // Test BOOTSTRAP_SERVERS that cannot be linked + Catalog kafka = + metalake.createCatalog( + NameIdentifier.of(METALAKE_NAME, catalogName), + Catalog.Type.MESSAGING, + PROVIDER, + "comment", + ImmutableMap.of( + BOOTSTRAP_SERVERS, + "192.0.2.1:9999", + CATALOG_BYPASS_PREFIX + AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, + "3000", + CATALOG_BYPASS_PREFIX + AdminClientConfig.DEFAULT_API_TIMEOUT_MS_CONFIG, + "3000")); + exception = + Assertions.assertThrows( + RuntimeException.class, + () -> + kafka + .asTopicCatalog() + .listTopics( + Namespace.ofTopic(METALAKE_NAME, catalogName, DEFAULT_SCHEMA_NAME))); + Assertions.assertTrue( + exception + .getMessage() + .contains("Timed out waiting for a node assignment. Call: listTopics"), + exception.getMessage()); } @Test diff --git a/catalogs/catalog-kafka/src/test/resources/log4j2.properties b/catalogs/catalog-kafka/src/test/resources/log4j2.properties index 890facc4ddf..afb89fe28f5 100644 --- a/catalogs/catalog-kafka/src/test/resources/log4j2.properties +++ b/catalogs/catalog-kafka/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfig appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Log files location property.logPath = ${sys:gravitino.log.path:-catalog-messaging-kafka/build/kafka-integration-test.log} diff --git a/catalogs/catalog-lakehouse-iceberg/src/test/resources/log4j2.properties b/catalogs/catalog-lakehouse-iceberg/src/test/resources/log4j2.properties index 3a0bcf1a5c4..cb1b9107700 100644 --- a/catalogs/catalog-lakehouse-iceberg/src/test/resources/log4j2.properties +++ b/catalogs/catalog-lakehouse-iceberg/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfig appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Log files location property.logPath = ${sys:gravitino.log.path:-catalog-lakehouse-iceberg/build/iceberg-integration-test.log} diff --git a/clients/client-java/src/test/java/resources/log4j2.properties b/clients/client-java/src/test/java/resources/log4j2.properties index 6beee06bcdc..b9e706603a0 100644 --- a/clients/client-java/src/test/java/resources/log4j2.properties +++ b/clients/client-java/src/test/java/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfigDemo appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Root logger level rootLogger.level = debug diff --git a/clients/client-java/src/test/resources/log4j2.properties b/clients/client-java/src/test/resources/log4j2.properties index 6beee06bcdc..b9e706603a0 100644 --- a/clients/client-java/src/test/resources/log4j2.properties +++ b/clients/client-java/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfigDemo appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Root logger level rootLogger.level = debug diff --git a/common/src/test/java/resources/log4j2.properties b/common/src/test/java/resources/log4j2.properties index 6beee06bcdc..b9e706603a0 100644 --- a/common/src/test/java/resources/log4j2.properties +++ b/common/src/test/java/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfigDemo appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Root logger level rootLogger.level = debug diff --git a/core/src/test/resources/log4j2.properties b/core/src/test/resources/log4j2.properties index 6beee06bcdc..b9e706603a0 100644 --- a/core/src/test/resources/log4j2.properties +++ b/core/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfigDemo appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Root logger level rootLogger.level = debug diff --git a/dev/docker/trino/conf/log4j2.properties b/dev/docker/trino/conf/log4j2.properties index 7f9ce0f52e2..d6e3418cde4 100644 --- a/dev/docker/trino/conf/log4j2.properties +++ b/dev/docker/trino/conf/log4j2.properties @@ -13,14 +13,14 @@ name = ConsoleLogConfig appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # File appender configuration appender.file.type = File appender.file.name = fileLogger appender.file.fileName = gravitino-trino-connector.log appender.file.layout.type = PatternLayout -appender.file.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.file.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Root logger level rootLogger.level = info diff --git a/integration-test/src/test/resources/log4j2.properties b/integration-test/src/test/resources/log4j2.properties index 5559c935426..d8b9313d07b 100644 --- a/integration-test/src/test/resources/log4j2.properties +++ b/integration-test/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfig appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Log files location property.logPath = ${sys:gravitino.log.path:-integration-test/build/integration-test.log} diff --git a/server/src/test/resources/log4j2.properties b/server/src/test/resources/log4j2.properties index 6beee06bcdc..b9e706603a0 100644 --- a/server/src/test/resources/log4j2.properties +++ b/server/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfigDemo appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Root logger level rootLogger.level = debug diff --git a/spark-connector/spark-connector/src/test/resources/log4j2.properties b/spark-connector/spark-connector/src/test/resources/log4j2.properties index 659ae2560a3..ce232cc5cf2 100644 --- a/spark-connector/spark-connector/src/test/resources/log4j2.properties +++ b/spark-connector/spark-connector/src/test/resources/log4j2.properties @@ -13,7 +13,7 @@ name = ConsoleLogConfig appender.console.type = Console appender.console.name = consoleLogger appender.console.layout.type = PatternLayout -appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.console.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Log files location property.logPath = ${sys:gravitino.log.path:-integration-test/build/integration-test.log} @@ -23,7 +23,7 @@ appender.file.type = File appender.file.name = fileLogger appender.file.fileName = ${logPath} appender.file.layout.type = PatternLayout -appender.file.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n +appender.file.layout.pattern = %d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L - %m%n # Root logger level rootLogger.level = info