RockteMQ-AI commented on code in PR #2644:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2644#discussion_r3876406557
##########
web/src/components/MessageQueryHistoryDrawer.tsx:
##########
@@ -109,6 +109,12 @@ const MessageQueryHistoryDrawer = ({
const traceColumns: ColumnsType<TraceQueryHistory> = [
{ title: 'Message ID', dataIndex: 'msgId', ellipsis: true },
{ title: 'Topic', dataIndex: 'topic', ellipsis: true },
+ {
+ title: '轨迹 Topic',
+ dataIndex: 'traceTopic',
Review Comment:
Column header mixes Chinese and English. If the project has an i18n
convention, consider using a translation key or keeping the header fully in one
language for consistency. Minor, not blocking.
##########
server/src/main/java/org/apache/rocketmq/studio/instance/message/MessageService.java:
##########
@@ -161,17 +165,22 @@ private void recordMessageQuery(String instanceId, String
topic, String msgId, S
}
}
- private void recordTraceQuery(String instanceId, String msgId, String
topic, TraceRecordVO result) {
+ private void recordTraceQuery(String instanceId, String msgId, String
topic, String traceTopic,
+ TraceRecordVO result) {
int nodeCount = result == null || result.getNodes() == null ? 0 :
result.getNodes().size();
int consumerCount = result == null || result.getConsumerStatus() ==
null ? 0
: result.getConsumerStatus().size();
try {
- queryHistoryService.recordTraceQuery(instanceId, msgId, topic,
nodeCount, consumerCount);
+ queryHistoryService.recordTraceQuery(instanceId, msgId, topic,
traceTopic, nodeCount, consumerCount);
} catch (RuntimeException failure) {
Review Comment:
normalizeOptional() is duplicated in QueryHistoryService with identical
logic. Consider extracting to a shared utility to avoid drift. Low priority.
##########
server/src/main/java/org/apache/rocketmq/studio/instance/message/QueryHistorySchemaMigration.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.studio.instance.message;
+
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.boot.ApplicationArguments;
+import org.springframework.boot.ApplicationRunner;
+import org.springframework.stereotype.Component;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+/**
+ * Adds the nullable trace-topic column to query-history tables created by
older Studio builds.
+ *
+ * <p>The main schema contains the column for fresh installations. Existing
installations do
+ * not run the schema script again, so the application must make this small
upgrade idempotently
+ * before the history repository starts reading or writing the new field.</p>
+ */
+@Slf4j
+@Component
+@RequiredArgsConstructor
+public class QueryHistorySchemaMigration implements ApplicationRunner {
+
+ private static final String TRACE_TABLE = "rmq_instance_trace";
+ private static final String TRACE_TOPIC_COLUMN = "trace_topic";
+ private static final String TRACE_TOPIC_DEFINITION = "VARCHAR(255)";
+
+ private final DataSource dataSource;
+
+ @Override
+ public void run(ApplicationArguments args) throws Exception {
+ try (Connection connection = dataSource.getConnection(); Statement
statement = connection.createStatement()) {
+ DatabaseMetaData metadata = connection.getMetaData();
+ String catalog = connection.getCatalog();
+ if (!hasTable(metadata, catalog, TRACE_TABLE)) {
+ // The regular schema initializer may be disabled in a custom
deployment. Do
+ // not fail startup here merely because there is no history
table to upgrade;
+ // the first history query will report the missing table in
the usual way.
+ log.debug("Skipping trace history migration because {} does
not exist", TRACE_TABLE);
+ return;
+ }
+ ensureTraceTopicColumn(metadata, catalog, statement);
+ }
+ }
+
+ private static void ensureTraceTopicColumn(DatabaseMetaData metadata,
String catalog,
+ Statement statement) throws
Exception {
+ if (hasColumn(metadata, catalog, TRACE_TABLE, TRACE_TOPIC_COLUMN)) {
+ return;
+ }
+ try {
+ log.info("Adding query-history column {}.{}", TRACE_TABLE,
TRACE_TOPIC_COLUMN);
Review Comment:
Race-condition handling for concurrent replicas is well done. One minor
note: VARCHAR(255) is reasonable for topic names, but if RocketMQ enforces a
specific max length elsewhere, consider aligning the column size with that
constraint for consistency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]