[GitHub] [skywalking-banyandb] hanahmily commented on a diff in pull request #265: Fix TopN reload issue

["hanahmily (via GitHub)" <gi...@apache.org>]

hanahmily commented on code in PR #265:
URL: https://github.com/apache/skywalking-banyandb/pull/265#discussion_r1168630727


##########
banyand/measure/metadata.go:
##########
@@ -115,6 +121,13 @@ func (sr *schemaRepo) OnAddOrUpdate(metadata schema.Metadata) {
 			})
 		}
 	case schema.KindTopNAggregation:
+		// createOrUpdate TopN schemas in advance
+		err := sr.createOrUpdateTopNMeasure(metadata.Spec.(*databasev1.TopNAggregation))

Review Comment:
   When the "TopnAggregation" is deleted, the corresponding measure should also be removed.
   
   



##########
pkg/test/measure/etcd.go:
##########
@@ -66,13 +71,80 @@ func PreloadSchema(e schema.Registry) error {
 		return errors.WithStack(err)
 	}
 	if err := loadSchema(topNAggregationDir, &databasev1.TopNAggregation{}, func(topN *databasev1.TopNAggregation) error {
-		return e.CreateTopNAggregation(context.TODO(), topN)
+		return multierr.Append(e.CreateTopNAggregation(context.TODO(), topN), createOrUpdateTopNMeasure(e, topN))
 	}); err != nil {
 		return errors.WithStack(err)
 	}
 	return nil
 }
 
+func createOrUpdateTopNMeasure(reg schema.Registry, topNSchema *databasev1.TopNAggregation) error {

Review Comment:
   Could you tell me why you're copying codes here? You have already created measures for processing topNAggregation creation events.
   
   If the issue involves resolving flaky scenarios, using the 'eventually' function may be helpful.
   
   



-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org