This is an automated email from the ASF dual-hosted git repository. stigahuang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 0d0a410cf65951d634f81ec14b474d663f9cf587 Author: stiga-huang <[email protected]> AuthorDate: Fri Feb 2 12:35:51 2024 +0800 IMPALA-12780: Only show non-default options in the catalog operations page The details shown in the catalog operations page are too verbose. For instance, for ExecDdlRequest, we show the string of TDdlQueryOptions: query_options=TDdlQueryOptions(sync_ddl:false, debug_action:, lock_max_wait_time_s:300, kudu_table_reserve_seconds:0) What really matters is the non-default options, e.g. if sync_ddl is set to true, it should be shown. This patch improves the details field to show only non-default options. Also wraps the content of Query Id and Details columns so they can fit into the table. Change-Id: Ie62d4b9e4d357e02764e7a62f4dc107de602e1a5 Reviewed-on: http://gerrit.cloudera.org:8080/20985 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- common/thrift/CatalogService.thrift | 6 +++ .../catalog/monitor/CatalogOperationTracker.java | 61 ++++++++++++++++------ www/catalog_operations.tmpl | 8 +-- 3 files changed, 54 insertions(+), 21 deletions(-) diff --git a/common/thrift/CatalogService.thrift b/common/thrift/CatalogService.thrift index 701f10641..1e4e9f5ea 100644 --- a/common/thrift/CatalogService.thrift +++ b/common/thrift/CatalogService.thrift @@ -99,6 +99,8 @@ struct TCatalogUpdateResult { } // Subset of query options passed to DDL operations +// When changing default values, CatalogOperationTracker#increment() should also be +// updated. struct TDdlQueryOptions { // True if SYNC_DDL is set in query options 1: required bool sync_ddl @@ -253,6 +255,8 @@ struct TUpdatedPartition { // with details on the result of the operation. Used to add partitions after executing // DML operations, and could potentially be used in the future to update column stats // after DML operations. +// When changing default values, CatalogOperationTracker#increment() should also be +// updated. // TODO: Rename this struct to something more descriptive. struct TUpdateCatalogRequest { 1: required CatalogServiceVersion protocol_version = CatalogServiceVersion.V2 @@ -295,6 +299,8 @@ struct TUpdateCatalogResponse { } // Parameters of REFRESH/INVALIDATE METADATA commands +// When changing default values, CatalogOperationTracker#increment() should also be +// updated. struct TResetMetadataRequest { 1: required CatalogServiceVersion protocol_version = CatalogServiceVersion.V2 diff --git a/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOperationTracker.java b/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOperationTracker.java index f66f0cfd7..a3b616e5d 100644 --- a/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOperationTracker.java +++ b/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOperationTracker.java @@ -18,13 +18,16 @@ package org.apache.impala.catalog.monitor; import com.google.common.base.Preconditions; +import org.apache.commons.lang3.StringUtils; import org.apache.impala.service.BackendConfig; import org.apache.impala.thrift.TCatalogServiceRequestHeader; import org.apache.impala.thrift.TDdlExecRequest; +import org.apache.impala.thrift.TDdlQueryOptions; import org.apache.impala.thrift.TDdlType; import org.apache.impala.thrift.TCatalogOpRecord; import org.apache.impala.thrift.TGetOperationUsageResponse; import org.apache.impala.thrift.TOperationUsageCounter; +import org.apache.impala.thrift.TQueryOptions; import org.apache.impala.thrift.TResetMetadataRequest; import org.apache.impala.thrift.TTableName; import org.apache.impala.thrift.TUniqueId; @@ -54,6 +57,7 @@ public final class CatalogOperationTracker { private static final Logger LOG = LoggerFactory.getLogger(CatalogOperationTracker.class); public final static CatalogOperationTracker INSTANCE = new CatalogOperationTracker(); + private static final TQueryOptions DEFAULT_QUERY_OPTIONS = new TQueryOptions(); // Keeps track of the on-going DDL operations CatalogDdlCounter catalogDdlCounter_; @@ -163,8 +167,23 @@ public final class CatalogOperationTracker { public void increment(TDdlExecRequest ddlRequest, Optional<TTableName> tTableName) { if (ddlRequest.isSetHeader()) { - String details = "query_options=" + ddlRequest.query_options.toString(); - addRecord(ddlRequest.getHeader(), getDdlType(ddlRequest), tTableName, details); + // Only show non-default options in the 'details' field. + TDdlQueryOptions options = ddlRequest.query_options; + List<String> nonDefaultOptions = new ArrayList<>(); + if (options.sync_ddl) nonDefaultOptions.add("sync_ddl=true"); + if (StringUtils.isNotEmpty(options.debug_action)) { + nonDefaultOptions.add("debug_action=" + options.debug_action); + } + if (options.lock_max_wait_time_s != DEFAULT_QUERY_OPTIONS.lock_max_wait_time_s) { + nonDefaultOptions.add("lock_max_wait_time_s=" + options.lock_max_wait_time_s); + } + if (options.kudu_table_reserve_seconds + != DEFAULT_QUERY_OPTIONS.kudu_table_reserve_seconds) { + nonDefaultOptions.add( + "kudu_table_reserve_seconds=" + options.kudu_table_reserve_seconds); + } + addRecord(ddlRequest.getHeader(), getDdlType(ddlRequest), tTableName, + StringUtils.join(nonDefaultOptions, ", ")); } catalogDdlCounter_.incrementOperation(ddlRequest.ddl_type, tTableName); } @@ -179,15 +198,20 @@ public final class CatalogOperationTracker { Optional<TTableName> tTableName = req.table_name != null ? Optional.of(req.table_name) : Optional.empty(); if (req.isSetHeader()) { - String details = "sync_ddl=" + req.sync_ddl + - ", want_minimal_response=" + req.getHeader().want_minimal_response + - ", refresh_updated_hms_partitions=" + req.refresh_updated_hms_partitions; - if (req.isSetDebug_action() && !req.debug_action.isEmpty()) { - details += ", debug_action=" + req.debug_action; + List<String> details = new ArrayList<>(); + if (req.sync_ddl) details.add("sync_ddl=true"); + if (req.header.want_minimal_response) { + details.add("want_minimal_response=true"); + } + if (req.refresh_updated_hms_partitions) { + details.add("refresh_updated_hms_partitions=true"); + } + if (StringUtils.isNotEmpty(req.debug_action)) { + details.add("debug_action=" + req.debug_action); } addRecord(req.getHeader(), CatalogResetMetadataCounter.getResetMetadataType(req, tTableName).name(), - tTableName, details); + tTableName, StringUtils.join(details, ", ")); } catalogResetMetadataCounter_.incrementOperation(req); } @@ -203,20 +227,23 @@ public final class CatalogOperationTracker { Optional<TTableName> tTableName = Optional.of(new TTableName(req.db_name, req.target_table)); if (req.isSetHeader()) { - String details = "sync_ddl=" + req.sync_ddl + - ", is_overwrite=" + req.is_overwrite + - ", transaction_id=" + req.transaction_id + - ", write_id=" + req.write_id + - ", num_of_updated_partitions=" + req.getUpdated_partitionsSize(); + List<String> details = new ArrayList<>(); + details.add("#partitions=" + req.getUpdated_partitionsSize()); + if (req.sync_ddl) details.add("sync_ddl=true"); + if (req.is_overwrite) details.add("is_overwrite=true"); + if (req.transaction_id > 0) { + details.add("transaction_id=" + req.transaction_id); + } + if (req.write_id > 0) details.add("write_id=" + req.write_id); if (req.isSetIceberg_operation()) { - details += ", iceberg_operation=" + req.iceberg_operation.operation; + details.add("iceberg_operation=" + req.iceberg_operation.operation); } - if (req.isSetDebug_action() && !req.debug_action.isEmpty()) { - details += ", debug_action=" + req.debug_action; + if (StringUtils.isNotEmpty(req.debug_action)) { + details.add("debug_action=" + req.debug_action); } addRecord(req.getHeader(), CatalogFinalizeDmlCounter.getDmlType(req.getHeader().redacted_sql_stmt).name(), - tTableName, details); + tTableName, StringUtils.join(details, ", ")); } catalogFinalizeDmlCounter_.incrementOperation(req); } diff --git a/www/catalog_operations.tmpl b/www/catalog_operations.tmpl index 42d8c148b..c00b1bc14 100644 --- a/www/catalog_operations.tmpl +++ b/www/catalog_operations.tmpl @@ -104,7 +104,7 @@ under the License. {{#inflight_catalog_operations}} <tr> <td>{{thread_id}}</td> - <td>{{query_id}}</td> + <td style="min-width:150px;word-break:break-all;">{{query_id}}</td> <td>{{client_ip}}</td> <td>{{coordinator}}</td> <td>{{catalog_op_name}}</td> @@ -113,7 +113,7 @@ under the License. <td>{{start_time}}</td> <td>{{duration}}</td> <td>{{status}}</td> - <td>{{details}}</td> + <td style="min-width:240px;word-break:break-all;">{{details}}</td> </tr> {{/inflight_catalog_operations}} </table> @@ -149,7 +149,7 @@ under the License. {{#finished_catalog_operations}} <tr> <td>{{thread_id}}</td> - <td>{{query_id}}</td> + <td style="min-width:150px;word-break:break-all;">{{query_id}}</td> <td>{{client_ip}}</td> <td>{{coordinator}}</td> <td>{{catalog_op_name}}</td> @@ -159,7 +159,7 @@ under the License. <td>{{finish_time}}</td> <td>{{duration}}</td> <td>{{status}}</td> - <td>{{details}}</td> + <td style="word-break:break-all;">{{details}}</td> </tr> {{/finished_catalog_operations}} </table>
