You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "zesiar0 (via GitHub)" <gi...@apache.org> on 2024/02/03 13:43:48 UTC
[PR] get entity value from series index when query stream data [skywalking-banyandb]
zesiar0 opened a new pull request, #386:
URL: https://github.com/apache/skywalking-banyandb/pull/386
- [x] correct encode tag value method
- [x] when query stream data, get entity value from series index
--
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
Re: [PR] get entity value from series index when query stream data [skywalking-banyandb]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #386:
URL: https://github.com/apache/skywalking-banyandb/pull/386#issuecomment-1925446058
## [Codecov](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `246 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`a7e483f`)](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/a7e483f97e9f28feb7cb8530c138403b3b2ab008?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 47.31% compared to head [(`c1ae510`)](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 46.86%.
> Report is 1 commits behind head on main.
| [Files](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [banyand/stream/query.go](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YmFueWFuZC9zdHJlYW0vcXVlcnkuZ28=) | 0.00% | [122 Missing :warning: ](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [banyand/measure/query.go](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YmFueWFuZC9tZWFzdXJlL3F1ZXJ5Lmdv) | 3.44% | [56 Missing :warning: ](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pkg/query/logical/measure/topn\_analyzer.go](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL3F1ZXJ5L2xvZ2ljYWwvbWVhc3VyZS90b3BuX2FuYWx5emVyLmdv) | 0.00% | [18 Missing :warning: ](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [banyand/measure/topn.go](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YmFueWFuZC9tZWFzdXJlL3RvcG4uZ28=) | 0.00% | [16 Missing :warning: ](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [banyand/measure/write.go](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YmFueWFuZC9tZWFzdXJlL3dyaXRlLmdv) | 0.00% | [13 Missing :warning: ](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [banyand/stream/write.go](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YmFueWFuZC9zdHJlYW0vd3JpdGUuZ28=) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [banyand/stream/part.go](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YmFueWFuZC9zdHJlYW0vcGFydC5nbw==) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [banyand/stream/tstable.go](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YmFueWFuZC9zdHJlYW0vdHN0YWJsZS5nbw==) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [banyand/stream/iter.go](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YmFueWFuZC9zdHJlYW0vaXRlci5nbw==) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pkg/query/logical/measure/topn\_plan\_localscan.go](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL3F1ZXJ5L2xvZ2ljYWwvbWVhc3VyZS90b3BuX3BsYW5fbG9jYWxzY2FuLmdv) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #386 +/- ##
==========================================
- Coverage 47.31% 46.86% -0.46%
==========================================
Files 172 172
Lines 21250 21470 +220
==========================================
+ Hits 10054 10061 +7
- Misses 10356 10570 +214
+ Partials 840 839 -1
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/386?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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
Re: [PR] get entity value from series index when query stream data [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily commented on code in PR #386:
URL: https://github.com/apache/skywalking-banyandb/pull/386#discussion_r1493856997
##########
banyand/measure/write.go:
##########
@@ -148,6 +149,14 @@ func (w *writeCallback) handle(dst map[string]*dataPointsInGroup, writeEvent *me
} else {
tagValue = tagFamily.Tags[j]
}
+ tagFamiliesForIndexWrite[i].values = append(tagFamiliesForIndexWrite[i].values, encodeTagValue(
Review Comment:
It is better to merge the encoding operation in lines 152 and 160 and encode the tag value only once.
##########
banyand/stream/query.go:
##########
@@ -161,6 +175,46 @@ func (qr *queryResult) Pull() *pbv1.StreamResult {
qr.data = append(qr.data[:i], qr.data[i+1:]...)
i--
}
+ if qr.schema.GetEntity() == nil || len(qr.schema.GetEntity().GetTagNames()) == 0 {
+ continue
+ }
+ sidIndex := qr.sidToIndex[qr.data[i].bm.seriesID]
+ series := qr.seriesList[sidIndex]
+ entityMap := make(map[string]int)
Review Comment:
It's better to move it in queryResult and fill it in Query function.
##########
banyand/stream/query.go:
##########
@@ -41,6 +43,15 @@ type queryOptions struct {
maxTimestamp int64
}
+func mustEncodeTagValue(name string, tagType databasev1.TagType, tagValue *modelv1.TagValue, num int) [][]byte {
+ values := make([][]byte, num)
+ nv := encodeTagValue(name, tagType, tagValue)
+ for i := 0; i < num; i++ {
+ values[i] = nv.marshal()
Review Comment:
Moving the marshaling process outside the loop will help improve performance and efficiency.
##########
banyand/stream/write.go:
##########
@@ -134,7 +139,7 @@ func (w *writeCallback) handle(dst map[string]*elementsInGroup, writeEvent *stre
tagFamilySpec.Tags[j].Type,
tagValue,
))
- if tagFamilySpec.Tags[j].IndexedOnly {
+ if tagFamilySpec.Tags[j].IndexedOnly || entityMap[tagFamilySpec.Tags[j].Name] {
continue
}
tagFamilies[i].values = append(tagFamilies[i].values, encodeTagValue(
Review Comment:
As I mentioned in the measure's "write.go", merging encoding tag operations is better.
##########
banyand/stream/query.go:
##########
@@ -342,6 +437,33 @@ func (s *stream) Sort(ctx context.Context, sso pbv1.StreamSortOptions) (ssr pbv1
seriesList = seriesList.Merge(sl)
}
+ entityMap := make(map[string]int)
+ for idx, entity := range s.schema.GetEntity().GetTagNames() {
+ entityMap[entity] = idx + 1
+ }
+ tagSpecIndex := make(map[string]*databasev1.TagSpec)
+ for _, tagFamilySpec := range s.schema.GetTagFamilies() {
+ for _, tagSpec := range tagFamilySpec.Tags {
+ tagSpecIndex[tagSpec.GetName()] = tagSpec
+ }
+ }
+ tagProjIndex := make(map[string]partition.TagLocator)
+ for i, tagFamilyProj := range sso.TagProjection {
+ for j, tagProj := range tagFamilyProj.Names {
+ if entityMap[tagProj] == 0 {
+ continue
+ }
+ tagProjIndex[tagProj] = partition.TagLocator{
+ FamilyOffset: i,
+ TagOffset: j,
+ }
+ }
+ }
+ sidToIndex := make(map[common.SeriesID]int)
+ for idx, series := range seriesList {
+ sidToIndex[series.ID] = idx
+ }
+
Review Comment:
Duplicated codes. It's better to abstract a function to generate them.
--
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
Re: [PR] get entity value from series index when query stream data [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily commented on code in PR #386:
URL: https://github.com/apache/skywalking-banyandb/pull/386#discussion_r1482240006
##########
banyand/measure/query.go:
##########
@@ -393,6 +393,9 @@ func (qr *queryResult) Pull() *pbv1.MeasureResult {
offset := qr.tagNameIndex[tagProj]
tagFamilySpec := qr.schema.GetTagFamilies()[offset.FamilyOffset]
tagSpec := tagFamilySpec.GetTags()[offset.TagOffset]
+ if tagSpec.IndexedOnly {
+ continue
+ }
Review Comment:
It would be better for you to update write.go instead of query.go. "IndexOnly" means the tag is not stored in the file.
--
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
Re: [PR] get entity value from series index when query stream data [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily merged PR #386:
URL: https://github.com/apache/skywalking-banyandb/pull/386
--
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