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]

Reply via email to