[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #10448: Integrate BanyanDB server-side TopN

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

lujiajing1126 commented on code in PR #10448:
URL: https://github.com/apache/skywalking/pull/10448#discussion_r1117939709


##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBAggregationQueryDAO.java:
##########
@@ -51,6 +52,43 @@ public BanyanDBAggregationQueryDAO(BanyanDBStorageClient client) {
     public List<SelectedRecord> sortMetrics(TopNCondition condition, String valueColumnName, Duration duration, List<KeyValue> additionalConditions) throws IOException {
         final String modelName = condition.getName();
         final TimestampRange timestampRange = new TimestampRange(duration.getStartTimestamp(), duration.getEndTimestamp());
+        // fast-path: BanyanDB server-side TopN support
+        MetadataRegistry.Schema schema = MetadataRegistry.INSTANCE.findMetadata(modelName, duration.getStep());
+        if (schema == null) {
+            throw new IOException("schema is not registered");
+        }
+
+        MetadataRegistry.ColumnSpec spec = schema.getSpec(valueColumnName);
+        if (spec == null) {
+            throw new IOException("field spec is not registered");
+        }
+
+        if (schema.hasTopNAggregation()) {

Review Comment:
   > In which case, there is no server-side pre-calculated aggregation?
   
   For those metrics that do not have field.



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