You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/12/19 07:26:43 UTC
[GitHub] [skywalking] hanahmily commented on a diff in pull request #10181: Refactor Topology query for BanyanDB
hanahmily commented on code in PR #10181:
URL: https://github.com/apache/skywalking/pull/10181#discussion_r1051880548
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/measure/BanyanDBTopologyQueryDAO.java:
##########
@@ -74,43 +78,34 @@ public List<Call.CallDetail> loadServiceRelationDetectedAtClientSide(Duration du
throw new UnexpectedException("Service id is empty");
}
- List<QueryBuilder<MeasureQuery>> queryBuilderList = buildServiceRelationsQueries(serviceIds);
+ QueryBuilder<MeasureQuery> queryBuilder = buildServiceRelationsQuery(serviceIds);
- return queryServiceRelation(duration, queryBuilderList, DetectPoint.CLIENT);
+ return queryServiceRelation(duration, queryBuilder, DetectPoint.CLIENT);
}
@Override
public List<Call.CallDetail> loadServiceRelationsDetectedAtServerSide(Duration duration) throws IOException {
- return queryServiceRelation(duration, Collections.singletonList(emptyMeasureQuery()), DetectPoint.SERVER);
+ return queryServiceRelation(duration, emptyMeasureQuery(), DetectPoint.SERVER);
}
@Override
public List<Call.CallDetail> loadServiceRelationDetectedAtClientSide(Duration duration) throws IOException {
- return queryServiceRelation(duration, Collections.singletonList(emptyMeasureQuery()), DetectPoint.CLIENT);
+ return queryServiceRelation(duration, emptyMeasureQuery(), DetectPoint.CLIENT);
}
- private List<QueryBuilder<MeasureQuery>> buildServiceRelationsQueries(List<String> serviceIds) {
- List<QueryBuilder<MeasureQuery>> queryBuilderList = new ArrayList<>(serviceIds.size());
- for (final String serviceId : serviceIds) {
- queryBuilderList.add(new QueryBuilder<MeasureQuery>() {
- @Override
- protected void apply(MeasureQuery query) {
- query.and(eq(ServiceRelationServerSideMetrics.SOURCE_SERVICE_ID, serviceId));
- }
- });
-
- queryBuilderList.add(new QueryBuilder<MeasureQuery>() {
- @Override
- protected void apply(MeasureQuery query) {
- query.and(eq(ServiceRelationServerSideMetrics.DEST_SERVICE_ID, serviceId));
- }
- });
- }
- return queryBuilderList;
+ private QueryBuilder<MeasureQuery> buildServiceRelationsQuery(List<String> serviceIds) {
+ return new QueryBuilder<MeasureQuery>() {
+ @Override
+ protected void apply(MeasureQuery query) {
+ query.or(in(ServiceRelationServerSideMetrics.SOURCE_SERVICE_ID, serviceIds))
+ .or(in(ServiceRelationServerSideMetrics.DEST_SERVICE_ID, serviceIds));
+ query.groupBy(Sets.newHashSet(Metrics.ENTITY_ID, ServiceRelationServerSideMetrics.COMPONENT_IDS));
Review Comment:
The same as the comment left at line:311
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/measure/BanyanDBTopologyQueryDAO.java:
##########
@@ -303,25 +300,35 @@ List<Call.CallDetail> queryProcessRelation(Duration duration,
timestampRange = new TimestampRange(TimeBucket.getTimestamp(startTB), TimeBucket.getTimestamp(endTB));
}
final String modelName = detectPoint == DetectPoint.SERVER ? ProcessRelationServerSideMetrics.INDEX_NAME :
- ProcessRelationClientSideMetrics.INDEX_NAME;
- final Map<String, Call.CallDetail> callMap = new HashMap<>();
+ ProcessRelationClientSideMetrics.INDEX_NAME;
+
MeasureQueryResponse resp = query(modelName,
- ImmutableSet.of(
- Metrics.ENTITY_ID, ProcessRelationClientSideMetrics.COMPONENT_ID),
- Collections.emptySet(), timestampRange, new QueryBuilder<MeasureQuery>() {
- @Override
- protected void apply(MeasureQuery query) {
- query.and(eq(ProcessRelationServerSideMetrics.SERVICE_INSTANCE_ID, serviceInstanceId));
+ ImmutableSet.of(Metrics.ENTITY_ID, ProcessRelationClientSideMetrics.COMPONENT_ID),
+ Collections.emptySet(), timestampRange, new QueryBuilder<MeasureQuery>() {
+ @Override
+ protected void apply(MeasureQuery query) {
+ query.and(eq(ProcessRelationServerSideMetrics.SERVICE_INSTANCE_ID, serviceInstanceId));
+ query.groupBy(Sets.newHashSet(Metrics.ENTITY_ID, ProcessRelationServerSideMetrics.COMPONENT_ID));
Review Comment:
* The HashSet doesn't maintain the order of tags. Could you opt for a sortable set instead?
* Please add `component_id` to the measure's entity list. The order of tags in groupBy should match the order of entities. That will help improve the memory overhead
--
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