swuferhong commented on code in PR #2282:
URL: https://github.com/apache/fluss/pull/2282#discussion_r2652802309
##########
fluss-server/src/main/java/org/apache/fluss/server/kv/KvManager.java:
##########
@@ -304,6 +312,9 @@ public void dropKv(TableBucket tableBucket) {
if (dropKvTablet != null) {
TablePath tablePath = dropKvTablet.getTablePath();
try {
+ // Unregister RocksDB metrics from TableMetricGroup
+ serverMetricGroup.removeTableBucketMetricGroup(tablePath,
tableBucket);
Review Comment:
why not call `serverMetricsGroup.unregisterRocksDBMetrics` as the comment
said?
##########
fluss-server/src/main/java/org/apache/fluss/server/metrics/group/TableMetricGroup.java:
##########
@@ -236,17 +243,159 @@ public BucketMetricGroup addBucketMetricGroup(
public void removeBucketMetricGroup(TableBucket tableBucket) {
BucketMetricGroup metricGroup = buckets.remove(tableBucket);
- metricGroup.close();
+ if (metricGroup != null) {
+ metricGroup.close();
+ }
+ // Also remove RocksDB metrics if exists
+ RocksDBMetrics rocksDBMetrics = rocksDBMetricsMap.remove(tableBucket);
+ if (rocksDBMetrics != null) {
+ try {
+ rocksDBMetrics.close();
+ } catch (Exception e) {
+ // Ignore close errors
+ }
+ }
+ }
+
+ /**
+ * Register RocksDB metrics for a bucket. This allows table-level
aggregation without
+ * registering bucket-level metrics.
+ *
+ * @param tableBucket the table bucket
+ * @param rocksDBMetrics the RocksDB metrics accessor
+ */
+ public void registerRocksDBMetrics(TableBucket tableBucket, RocksDBMetrics
rocksDBMetrics) {
+ rocksDBMetricsMap.put(tableBucket, rocksDBMetrics);
+ }
+
+ /**
+ * Unregister RocksDB metrics for a bucket.
+ *
+ * @param tableBucket the table bucket
+ */
+ public void unregisterRocksDBMetrics(TableBucket tableBucket) {
Review Comment:
This method is never used?
--
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]
