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


##########
banyand/measure/measure_write.go:
##########
@@ -121,6 +122,9 @@ func (s *measure) write(md *commonv1.Metadata, shardID 
common.ShardID, entity []
                _ = wp.Close()
                return err
        }
+       if isVirtualMeasure {

Review Comment:
   You don't have to add this flag. The `measure` stores topn's results don't 
refer to any `indexRules`. It won't generate any indices. 
   
   



##########
banyand/measure/measure_write.go:
##########
@@ -39,17 +39,18 @@ import (
 
 var errMalformedElement = errors.New("element is malformed")
 
-func (s *measure) write(md *commonv1.Metadata, shardID common.ShardID, entity 
[]byte, entityValues tsdb.EntityValues, value *measurev1.DataPointValue) error {
+func (s *measure) write(sm *databasev1.Measure, shardID common.ShardID, entity 
[]byte, entityValues tsdb.EntityValues,

Review Comment:
   Injecting a faked measure is a terrible design that will mess up the writing 
process. 
   You had better build and register a new `measure` to write topN's result. 



##########
banyand/measure/measure_write.go:
##########
@@ -60,7 +61,7 @@ func (s *measure) write(md *commonv1.Metadata, shardID 
common.ShardID, entity []
        if err != nil {
                return err
        }
-       ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+       ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Second)

Review Comment:
   Why do you change it to `TODO`? This context is the top-level, `Background` 
is better here. 



-- 
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