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/04/26 08:26:39 UTC
[GitHub] [skywalking-banyandb] hanahmily opened a new pull request, #110: Introduce gorilla encoder to measure
hanahmily opened a new pull request, #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110
Assemble gorilla encoder/decoder to integer fields of a measure.
fix https://github.com/apache/skywalking/issues/8847
This fixes the server-side of https://github.com/apache/skywalking/issues/8948 at the same time.
Signed-off-by: Gao Hongtao <ha...@gmail.com>
--
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
[GitHub] [skywalking-banyandb] lujiajing1126 commented on a diff in pull request #110: Introduce gorilla encoder to measure
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110#discussion_r858440347
##########
banyand/measure/measure.go:
##########
@@ -70,10 +73,14 @@ func (s *measure) Close() error {
return s.indexWriter.Close()
}
-func (s *measure) parseSpec() {
+func (s *measure) parseSpec() (err error) {
s.name, s.group = s.schema.GetMetadata().GetName(), s.schema.GetMetadata().GetGroup()
s.entityLocator = partition.NewEntityLocator(s.schema.GetTagFamilies(), s.schema.GetEntity())
s.maxObservedModRevision = pbv1.ParseMaxModRevision(s.indexRules)
+ if s.schema.Interval != "" {
Review Comment:
It is possible that the `Interval` is empty?
##########
banyand/measure/measure_write.go:
##########
@@ -196,8 +195,8 @@ func (w *writeCallback) Rev(message bus.Message) (resp bus.Message) {
return
}
-func familyIdentity(name string, flag byte) []byte {
- return bytes.Join([][]byte{[]byte(name), {flag}}, nil)
+func familyIdentity(name string, flag []byte) []byte {
+ return bytes.Join([][]byte{tsdb.Hash([]byte(name)), flag}, nil)
Review Comment:
It seems strange to use `bytes.Join` here?
##########
pkg/timestamp/duration.go:
##########
@@ -0,0 +1,53 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+package timestamp
+
+import (
+ "strconv"
+ "strings"
+ "time"
+)
+
+// ParseDuration parses a duration string.
+// A duration string is a possibly signed sequence of
+// decimal numbers, each with optional fraction and a unit suffix,
+// such as "300ms", "-1.5h" or "2h45m".
Review Comment:
Emmm, in which scenarios, we should support negative duration?
--
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
[GitHub] [skywalking-banyandb] hanahmily commented on a diff in pull request #110: Introduce gorilla encoder to measure
Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110#discussion_r858501269
##########
pkg/timestamp/duration.go:
##########
@@ -0,0 +1,53 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+package timestamp
+
+import (
+ "strconv"
+ "strings"
+ "time"
+)
+
+// ParseDuration parses a duration string.
+// A duration string is a possibly signed sequence of
+// decimal numbers, each with optional fraction and a unit suffix,
+// such as "300ms", "-1.5h" or "2h45m".
Review Comment:
No special scenario. It's an extension of the std time module, and I don't tend to remove the negative support.
--
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
[GitHub] [skywalking-banyandb] lujiajing1126 commented on a diff in pull request #110: Introduce gorilla encoder to measure
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110#discussion_r858539177
##########
banyand/measure/measure_write.go:
##########
@@ -196,8 +195,8 @@ func (w *writeCallback) Rev(message bus.Message) (resp bus.Message) {
return
}
-func familyIdentity(name string, flag byte) []byte {
- return bytes.Join([][]byte{[]byte(name), {flag}}, nil)
+func familyIdentity(name string, flag []byte) []byte {
+ return bytes.Join([][]byte{tsdb.Hash([]byte(name)), flag}, nil)
Review Comment:
> Any thoughts here? I applied this pattern everywhere.
more or less OK for me. Let's optimize later~
--
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
[GitHub] [skywalking-banyandb] hanahmily commented on a diff in pull request #110: Introduce gorilla encoder to measure
Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110#discussion_r859340698
##########
banyand/measure/measure.go:
##########
@@ -70,10 +73,14 @@ func (s *measure) Close() error {
return s.indexWriter.Close()
}
-func (s *measure) parseSpec() {
+func (s *measure) parseSpec() (err error) {
s.name, s.group = s.schema.GetMetadata().GetName(), s.schema.GetMetadata().GetGroup()
s.entityLocator = partition.NewEntityLocator(s.schema.GetTagFamilies(), s.schema.GetEntity())
s.maxObservedModRevision = pbv1.ParseMaxModRevision(s.indexRules)
+ if s.schema.Interval != "" {
Review Comment:
The interval is bound to the field encoding algorithm, not to the TTL.
--
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
[GitHub] [skywalking-banyandb] wu-sheng commented on a diff in pull request #110: Introduce gorilla encoder to measure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110#discussion_r858676042
##########
banyand/measure/measure.go:
##########
@@ -70,10 +73,14 @@ func (s *measure) Close() error {
return s.indexWriter.Close()
}
-func (s *measure) parseSpec() {
+func (s *measure) parseSpec() (err error) {
s.name, s.group = s.schema.GetMetadata().GetName(), s.schema.GetMetadata().GetGroup()
s.entityLocator = partition.NewEntityLocator(s.schema.GetTagFamilies(), s.schema.GetEntity())
s.maxObservedModRevision = pbv1.ParseMaxModRevision(s.indexRules)
+ if s.schema.Interval != "" {
Review Comment:
@lujiajing1126 All `*Traffic` data doesn't put time dimensionality into the ID. Basically, traffic is only TTL relative.
--
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
[GitHub] [skywalking-banyandb] hanahmily commented on a diff in pull request #110: Introduce gorilla encoder to measure
Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110#discussion_r858465285
##########
banyand/measure/measure.go:
##########
@@ -70,10 +73,14 @@ func (s *measure) Close() error {
return s.indexWriter.Close()
}
-func (s *measure) parseSpec() {
+func (s *measure) parseSpec() (err error) {
s.name, s.group = s.schema.GetMetadata().GetName(), s.schema.GetMetadata().GetGroup()
s.entityLocator = partition.NewEntityLocator(s.schema.GetTagFamilies(), s.schema.GetEntity())
s.maxObservedModRevision = pbv1.ParseMaxModRevision(s.indexRules)
+ if s.schema.Interval != "" {
Review Comment:
absolutely, the interval of `service_traffic` is empty.
--
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
[GitHub] [skywalking-banyandb] lujiajing1126 commented on a diff in pull request #110: Introduce gorilla encoder to measure
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110#discussion_r859299378
##########
banyand/measure/measure.go:
##########
@@ -70,10 +73,14 @@ func (s *measure) Close() error {
return s.indexWriter.Close()
}
-func (s *measure) parseSpec() {
+func (s *measure) parseSpec() (err error) {
s.name, s.group = s.schema.GetMetadata().GetName(), s.schema.GetMetadata().GetGroup()
s.entityLocator = partition.NewEntityLocator(s.schema.GetTagFamilies(), s.schema.GetEntity())
s.maxObservedModRevision = pbv1.ParseMaxModRevision(s.indexRules)
+ if s.schema.Interval != "" {
Review Comment:
> @lujiajing1126 All `*Traffic` data doesn't put time dimensionality into the ID. Basically, traffic is only TTL relative.
Is that because we don't have any `field` for `*Traffic`?
Since interval is mostly for GORILLA encoding, it is meaningless in such scenarios
--
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
[GitHub] [skywalking-banyandb] hanahmily commented on a diff in pull request #110: Introduce gorilla encoder to measure
Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110#discussion_r858498790
##########
banyand/measure/measure_write.go:
##########
@@ -196,8 +195,8 @@ func (w *writeCallback) Rev(message bus.Message) (resp bus.Message) {
return
}
-func familyIdentity(name string, flag byte) []byte {
- return bytes.Join([][]byte{[]byte(name), {flag}}, nil)
+func familyIdentity(name string, flag []byte) []byte {
+ return bytes.Join([][]byte{tsdb.Hash([]byte(name)), flag}, nil)
Review Comment:
Any thoughts here? I applied this pattern everywhere.
--
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
[GitHub] [skywalking-banyandb] lujiajing1126 commented on a diff in pull request #110: Introduce gorilla encoder to measure
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110#discussion_r858538188
##########
banyand/measure/measure.go:
##########
@@ -70,10 +73,14 @@ func (s *measure) Close() error {
return s.indexWriter.Close()
}
-func (s *measure) parseSpec() {
+func (s *measure) parseSpec() (err error) {
s.name, s.group = s.schema.GetMetadata().GetName(), s.schema.GetMetadata().GetGroup()
s.entityLocator = partition.NewEntityLocator(s.schema.GetTagFamilies(), s.schema.GetEntity())
s.maxObservedModRevision = pbv1.ParseMaxModRevision(s.indexRules)
+ if s.schema.Interval != "" {
Review Comment:
> absolutely, the interval of `service_traffic` is empty.
No. I suppose for `service_traffic`, it is `Minute`.
--
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
[GitHub] [skywalking-banyandb] lujiajing1126 merged pull request #110: Introduce gorilla encoder to measure
Posted by GitBox <gi...@apache.org>.
lujiajing1126 merged PR #110:
URL: https://github.com/apache/skywalking-banyandb/pull/110
--
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