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 2021/06/07 02:34:13 UTC

[GitHub] [skywalking-banyandb] hanahmily opened a new pull request #9: Add API for trace series, index rule and index rule binding

hanahmily opened a new pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9


   This PR intends:
   
    1. Introduce schema definition: trace series, index rule, and index rule binding.
    2. Add pre-defined skywalking schema objects.
    3. Add interfaces to the Series module for retrieving schema objects.
   
   You might notice that I added `service_name`, `service_instance_name` and  `endpoint_name` to skywalking TraceSeries(https://github.com/apache/skywalking-banyandb/compare/api-schema?expand=1#diff-9d5ac9ffaa5d323e02fdf127c4977aecc0e9c31c6de5ecff792f7e81768b31dfR23-R34), but didn't add them to the IndexRule correspondingly. They will be processed by the `MetaSeries` which will be introduced in the following up PRs. The  `MetaSereis` index service/instance/endpoint name to id. 
   
   @lujiajing1126 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r648006109



##########
File path: api/fbs/v1/schema.fbs
##########
@@ -27,21 +27,50 @@ struct Duration {
     unit:DurationUint;
 }
 
+enum FieldType:byte { String = 0, Int = 1, StringArray = 2, IntArray = 3 }
+
+table FieldSpec {
+    name:string;
+    type:FieldType;
+}
+
+table TraceStateMap {
+    field:string;
+    val_success:string;
+    val_error:string;
+}
+
+// The key in TraceFieldMap are reserved by trace series engine. Their corresponding value is the Fields or 
+// the combination of Fields
+table TraceFieldMap {
+    // trace_id the unique identity of a single trace
+    trace_id:string;
+    // series_id groups entites into a storage bucket
+    series_id:[string];
+    // state indicates the trace is "success" or "error"
+    state:TraceStateMap;
+}
+
 // TraceSeries represents a trace storage object
 table TraceSeries {
     // metadata is the identity of a trace series
     metadata:Metadata;
+    // fields defines accetped fields
+    fields:[FieldSpec];
+    // reserved_fields_map indicates how to index reserved fields to ingested fields
+    reserved_fields_map:TraceFieldMap;
     // duration determines how long a TraceSeries keeps its data
     duration:Duration;
-    // updated_at indicates when the TraceSeries is updated
-    updated_at:uint64;
+    // updated_nanoseconds_at indicates when the TraceSeries is updated
+    updated_nanoseconds_at:uint64;
 }
 
 // Catalog refers to a placement contains objects belonged to a particular data type
 enum Catalog:byte { Trace = 0, Log = 1, Metric = 2 }
 
 // IndexType determine the index structure under the hood
-enum IndexType:byte { Text = 0, Numerical = 1, ID = 2}
+// Fields with SeriesInternal type is reserved by Series module, would not be indexed by Index module.
+enum IndexType:byte { Text = 0, Numerical = 1, ID = 2, MultiText = 3, MultiNumberical = 4, SeriesInternal = 5 }
 
 // IndexObject defines who should be indexed.
 table IndexObject {

Review comment:
       user will input the trace series, in the skywalking scenario, the name is `sw` and the group is `default`. The combination of them is the metadata of trace series. Now you have two paths to get the index name:
   
   1. Using methods in `SchemaRepo`. Finding IndexRuleBinding by subject(above trace series) is the first step. Then you can get the desired IndexRule by the `rule_ref` in an IndexRuleBinding.
   
   2. A sugar method in `IndexFilter`. `RulesBySubject` provides a similar querying function as indicated in the first option. You can use it to find the IndexRule by a TraceSeries's metadata.
   
   The indexName of `Repo.Search` is the metadata of IndexRule. But I should twist the signature of this method to support a global index name.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r647939601



##########
File path: banyand/series/series.go
##########
@@ -22,29 +22,66 @@ import (
 	"context"
 
 	"github.com/apache/skywalking-banyandb/api/data"
+	v1 "github.com/apache/skywalking-banyandb/api/fbs/v1"
+	"github.com/apache/skywalking-banyandb/banyand/series/schema"
 	"github.com/apache/skywalking-banyandb/banyand/storage"
 	"github.com/apache/skywalking-banyandb/pkg/run"
 )
 
-type Trace interface {
+// TraceState represents the state of a trace link
+type TraceState int
+
+var (
+	TraceStateSuccess = 0
+	TraceStateError   = 1
+)
+
+//ScanOptions contain options
+//nolint
+type ScanOptions struct {
+	projection []string
+	state      TraceState
+	limit      uint32
+}
+
+//TraceRepo contains trace and entity data
+type TraceRepo interface {
+	//FetchTrace returns data.Trace by traceID
 	FetchTrace(traceID string) (data.Trace, error)
-	FetchEntity(chunkIDs []string, fields []string) ([]data.Entity, error)
-	ScanEntity(startTime, endTime uint64, fields []string) ([]data.Entity, error)
+	//FetchEntity returns data.Entity by ChunkID
+	FetchEntity(chunkIDs []string, opt ScanOptions) ([]data.Entity, error)
+	//ScanEntity returns data.Entity between a duration by ScanOptions
+	ScanEntity(startTime, endTime uint64, opt ScanOptions) ([]data.Entity, error)
 }
 
+//UniModel combines Trace, Metric and Log repositories into a union interface
 type UniModel interface {
-	Trace
+	TraceRepo
 }
 
+//SchemaRepo contains schema definition
+type SchemaRepo interface {
+	TraceSeries() schema.TraceSeries
+	IndexRule() schema.IndexRule
+	IndexRuleBinding() schema.IndexRuleBinding
+}
+
+//IndexFilter provides methods to find a specific index related objects
+type IndexFilter interface {
+	//RulesBySubject fetches IndexRule by Series defined in IndexRuleBinding
+	RulesBySubject(ctx context.Context, subject v1.Series) ([]v1.IndexRule, error)

Review comment:
       A single field might belong to several index rules, for example, a field is indexed separately or belongs to a combinate index. so the returned rule should be a vector instead of a single item.
   
   
   
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r647937241



##########
File path: banyand/series/series.go
##########
@@ -22,29 +22,66 @@ import (
 	"context"
 
 	"github.com/apache/skywalking-banyandb/api/data"
+	v1 "github.com/apache/skywalking-banyandb/api/fbs/v1"
+	"github.com/apache/skywalking-banyandb/banyand/series/schema"
 	"github.com/apache/skywalking-banyandb/banyand/storage"
 	"github.com/apache/skywalking-banyandb/pkg/run"
 )
 
-type Trace interface {
+// TraceState represents the state of a trace link
+type TraceState int
+
+var (
+	TraceStateSuccess = 0
+	TraceStateError   = 1
+)
+
+//ScanOptions contain options
+//nolint
+type ScanOptions struct {
+	projection []string
+	state      TraceState
+	limit      uint32
+}
+
+//TraceRepo contains trace and entity data
+type TraceRepo interface {
+	//FetchTrace returns data.Trace by traceID
 	FetchTrace(traceID string) (data.Trace, error)
-	FetchEntity(chunkIDs []string, fields []string) ([]data.Entity, error)
-	ScanEntity(startTime, endTime uint64, fields []string) ([]data.Entity, error)
+	//FetchEntity returns data.Entity by ChunkID
+	FetchEntity(chunkIDs []string, opt ScanOptions) ([]data.Entity, error)

Review comment:
       `uint64`




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] lujiajing1126 commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r648107486



##########
File path: banyand/index/index.go
##########
@@ -31,7 +31,7 @@ type Condition struct {
 }
 
 type Repo interface {
-	Search(indexName string, startTime, endTime uint64, conditions []Condition) ([]common.ChunkID, error)
+	Search(index common.Metadata, startTime, endTime uint64, conditions []Condition) ([]common.ChunkID, error)

Review comment:
       Would you complete this `Condition` struct in this PR or maybe the following ones?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] lujiajing1126 commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r647945652



##########
File path: api/fbs/v1/schema.fbs
##########
@@ -27,21 +27,50 @@ struct Duration {
     unit:DurationUint;
 }
 
+enum FieldType:byte { String = 0, Int = 1, StringArray = 2, IntArray = 3 }
+
+table FieldSpec {
+    name:string;
+    type:FieldType;
+}
+
+table TraceStateMap {
+    field:string;
+    val_success:string;
+    val_error:string;
+}
+
+// The key in TraceFieldMap are reserved by trace series engine. Their corresponding value is the Fields or 
+// the combination of Fields
+table TraceFieldMap {
+    // trace_id the unique identity of a single trace
+    trace_id:string;
+    // series_id groups entites into a storage bucket
+    series_id:[string];
+    // state indicates the trace is "success" or "error"
+    state:TraceStateMap;
+}
+
 // TraceSeries represents a trace storage object
 table TraceSeries {
     // metadata is the identity of a trace series
     metadata:Metadata;
+    // fields defines accetped fields
+    fields:[FieldSpec];
+    // reserved_fields_map indicates how to index reserved fields to ingested fields
+    reserved_fields_map:TraceFieldMap;
     // duration determines how long a TraceSeries keeps its data
     duration:Duration;
-    // updated_at indicates when the TraceSeries is updated
-    updated_at:uint64;
+    // updated_nanoseconds_at indicates when the TraceSeries is updated
+    updated_nanoseconds_at:uint64;
 }
 
 // Catalog refers to a placement contains objects belonged to a particular data type
 enum Catalog:byte { Trace = 0, Log = 1, Metric = 2 }
 
 // IndexType determine the index structure under the hood
-enum IndexType:byte { Text = 0, Numerical = 1, ID = 2}
+// Fields with SeriesInternal type is reserved by Series module, would not be indexed by Index module.
+enum IndexType:byte { Text = 0, Numerical = 1, ID = 2, MultiText = 3, MultiNumberical = 4, SeriesInternal = 5 }
 
 // IndexObject defines who should be indexed.
 table IndexObject {

Review comment:
       > You can leverage methods in `IndexFilter` to get desired index rule.
   
   Oh, I see the metadata in the `IndexRule`. But do we need a `group` here? Or indexRule's `name` is globally unique?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r646236460



##########
File path: banyand/series/schema/sw/trace_series.json
##########
@@ -0,0 +1,85 @@
+{

Review comment:
       @wu-sheng @Fine0830  This's the pre-defined schema for writing and querying as we discussed in the write API PR.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r648176425



##########
File path: banyand/index/index.go
##########
@@ -31,7 +31,7 @@ type Condition struct {
 }
 
 type Repo interface {
-	Search(indexName string, startTime, endTime uint64, conditions []Condition) ([]common.ChunkID, error)
+	Search(index common.Metadata, startTime, endTime uint64, conditions []Condition) ([]common.ChunkID, error)

Review comment:
       I would like to left it to you to complete it . 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] lujiajing1126 edited a comment on pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
lujiajing1126 edited a comment on pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#issuecomment-857520949


   Shall we be worry about the performance hits caused by dereferencing the pointers to structs such as `Metadata`, `Metadata.Spec` etc.
   
   I found that almost all APIs are using values instead of pointers for structs.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r648082340



##########
File path: banyand/series/series.go
##########
@@ -22,29 +22,66 @@ import (
 	"context"
 
 	"github.com/apache/skywalking-banyandb/api/data"
+	v1 "github.com/apache/skywalking-banyandb/api/fbs/v1"
+	"github.com/apache/skywalking-banyandb/banyand/series/schema"
 	"github.com/apache/skywalking-banyandb/banyand/storage"
 	"github.com/apache/skywalking-banyandb/pkg/run"
 )
 
-type Trace interface {
+// TraceState represents the state of a trace link
+type TraceState int
+
+var (
+	TraceStateSuccess = 0
+	TraceStateError   = 1
+)
+
+//ScanOptions contain options
+//nolint
+type ScanOptions struct {
+	projection []string
+	state      TraceState
+	limit      uint32
+}
+
+//TraceRepo contains trace and entity data
+type TraceRepo interface {
+	//FetchTrace returns data.Trace by traceID
 	FetchTrace(traceID string) (data.Trace, error)
-	FetchEntity(chunkIDs []string, fields []string) ([]data.Entity, error)
-	ScanEntity(startTime, endTime uint64, fields []string) ([]data.Entity, error)
+	//FetchEntity returns data.Entity by ChunkID
+	FetchEntity(chunkIDs []string, opt ScanOptions) ([]data.Entity, error)
+	//ScanEntity returns data.Entity between a duration by ScanOptions
+	ScanEntity(startTime, endTime uint64, opt ScanOptions) ([]data.Entity, error)
 }
 
+//UniModel combines Trace, Metric and Log repositories into a union interface
 type UniModel interface {
-	Trace
+	TraceRepo
 }
 
+//SchemaRepo contains schema definition
+type SchemaRepo interface {
+	TraceSeries() schema.TraceSeries
+	IndexRule() schema.IndexRule
+	IndexRuleBinding() schema.IndexRuleBinding
+}
+
+//IndexFilter provides methods to find a specific index related objects
+type IndexFilter interface {
+	//RulesBySubject fetches IndexRule by Series defined in IndexRuleBinding
+	RulesBySubject(ctx context.Context, subject v1.Series) ([]v1.IndexRule, error)

Review comment:
       I add a new parameter to filter a specific field(s), pls take a look




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#issuecomment-857585646


   > Shall we be worry about the performance hits caused by dereferencing the pointers to structs such as `Metadata`, `Metadata.Spec` etc.
   > 
   > I found that almost all APIs are using values instead of pointers for structs.
   
   I prefer to pass by value, which is much cheaper than C. furthermore, immutable parameters are safer from bugs. 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] lujiajing1126 commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r646250765



##########
File path: banyand/series/series.go
##########
@@ -22,29 +22,66 @@ import (
 	"context"
 
 	"github.com/apache/skywalking-banyandb/api/data"
+	v1 "github.com/apache/skywalking-banyandb/api/fbs/v1"
+	"github.com/apache/skywalking-banyandb/banyand/series/schema"
 	"github.com/apache/skywalking-banyandb/banyand/storage"
 	"github.com/apache/skywalking-banyandb/pkg/run"
 )
 
-type Trace interface {
+// TraceState represents the state of a trace link
+type TraceState int
+
+var (
+	TraceStateSuccess = 0
+	TraceStateError   = 1
+)
+
+//ScanOptions contain options
+//nolint
+type ScanOptions struct {
+	projection []string
+	state      TraceState
+	limit      uint32
+}
+
+//TraceRepo contains trace and entity data
+type TraceRepo interface {

Review comment:
       Do we need `metadata` for methods in `TraceRepo`?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r647936952



##########
File path: api/fbs/v1/schema.fbs
##########
@@ -27,21 +27,50 @@ struct Duration {
     unit:DurationUint;
 }
 
+enum FieldType:byte { String = 0, Int = 1, StringArray = 2, IntArray = 3 }
+
+table FieldSpec {
+    name:string;
+    type:FieldType;
+}
+
+table TraceStateMap {
+    field:string;
+    val_success:string;
+    val_error:string;
+}
+
+// The key in TraceFieldMap are reserved by trace series engine. Their corresponding value is the Fields or 
+// the combination of Fields
+table TraceFieldMap {
+    // trace_id the unique identity of a single trace
+    trace_id:string;
+    // series_id groups entites into a storage bucket
+    series_id:[string];
+    // state indicates the trace is "success" or "error"
+    state:TraceStateMap;
+}
+
 // TraceSeries represents a trace storage object
 table TraceSeries {
     // metadata is the identity of a trace series
     metadata:Metadata;
+    // fields defines accetped fields
+    fields:[FieldSpec];
+    // reserved_fields_map indicates how to index reserved fields to ingested fields
+    reserved_fields_map:TraceFieldMap;
     // duration determines how long a TraceSeries keeps its data
     duration:Duration;
-    // updated_at indicates when the TraceSeries is updated
-    updated_at:uint64;
+    // updated_nanoseconds_at indicates when the TraceSeries is updated
+    updated_nanoseconds_at:uint64;
 }
 
 // Catalog refers to a placement contains objects belonged to a particular data type
 enum Catalog:byte { Trace = 0, Log = 1, Metric = 2 }
 
 // IndexType determine the index structure under the hood
-enum IndexType:byte { Text = 0, Numerical = 1, ID = 2}
+// Fields with SeriesInternal type is reserved by Series module, would not be indexed by Index module.
+enum IndexType:byte { Text = 0, Numerical = 1, ID = 2, MultiText = 3, MultiNumberical = 4, SeriesInternal = 5 }
 
 // IndexObject defines who should be indexed.
 table IndexObject {

Review comment:
       You can leverage methods in `IndexFilter` to get desired index rule.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] lujiajing1126 edited a comment on pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
lujiajing1126 edited a comment on pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#issuecomment-857520949


   Shall we be worry about the performance hits caused by dereferencing the pointers to structs such as `Metadata`, `Metadata.Spec` etc.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily merged pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily merged pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] lujiajing1126 commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r646298152



##########
File path: api/fbs/v1/schema.fbs
##########
@@ -27,21 +27,50 @@ struct Duration {
     unit:DurationUint;
 }
 
+enum FieldType:byte { String = 0, Int = 1, StringArray = 2, IntArray = 3 }
+
+table FieldSpec {
+    name:string;
+    type:FieldType;
+}
+
+table TraceStateMap {
+    field:string;
+    val_success:string;
+    val_error:string;
+}
+
+// The key in TraceFieldMap are reserved by trace series engine. Their corresponding value is the Fields or 
+// the combination of Fields
+table TraceFieldMap {
+    // trace_id the unique identity of a single trace
+    trace_id:string;
+    // series_id groups entites into a storage bucket
+    series_id:[string];
+    // state indicates the trace is "success" or "error"
+    state:TraceStateMap;
+}
+
 // TraceSeries represents a trace storage object
 table TraceSeries {
     // metadata is the identity of a trace series
     metadata:Metadata;
+    // fields defines accetped fields
+    fields:[FieldSpec];
+    // reserved_fields_map indicates how to index reserved fields to ingested fields
+    reserved_fields_map:TraceFieldMap;
     // duration determines how long a TraceSeries keeps its data
     duration:Duration;
-    // updated_at indicates when the TraceSeries is updated
-    updated_at:uint64;
+    // updated_nanoseconds_at indicates when the TraceSeries is updated
+    updated_nanoseconds_at:uint64;
 }
 
 // Catalog refers to a placement contains objects belonged to a particular data type
 enum Catalog:byte { Trace = 0, Log = 1, Metric = 2 }
 
 // IndexType determine the index structure under the hood
-enum IndexType:byte { Text = 0, Numerical = 1, ID = 2}
+// Fields with SeriesInternal type is reserved by Series module, would not be indexed by Index module.
+enum IndexType:byte { Text = 0, Numerical = 1, ID = 2, MultiText = 3, MultiNumberical = 4, SeriesInternal = 5 }
 
 // IndexObject defines who should be indexed.
 table IndexObject {

Review comment:
       An indexName may be necessary for the object of `IndexRule`? Otherwise how can we determine the index while searching with API in `index.go`?
   
   ```go
   type Repo interface {
   	Search(indexName string, startTime, endTime uint64, conditions []Condition) ([]common.ChunkID, error)
   }
   ```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] lujiajing1126 commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r647958417



##########
File path: api/fbs/v1/schema.fbs
##########
@@ -27,21 +27,50 @@ struct Duration {
     unit:DurationUint;
 }
 
+enum FieldType:byte { String = 0, Int = 1, StringArray = 2, IntArray = 3 }
+
+table FieldSpec {
+    name:string;
+    type:FieldType;
+}
+
+table TraceStateMap {
+    field:string;
+    val_success:string;
+    val_error:string;
+}
+
+// The key in TraceFieldMap are reserved by trace series engine. Their corresponding value is the Fields or 
+// the combination of Fields
+table TraceFieldMap {
+    // trace_id the unique identity of a single trace
+    trace_id:string;
+    // series_id groups entites into a storage bucket
+    series_id:[string];
+    // state indicates the trace is "success" or "error"
+    state:TraceStateMap;
+}
+
 // TraceSeries represents a trace storage object
 table TraceSeries {
     // metadata is the identity of a trace series
     metadata:Metadata;
+    // fields defines accetped fields
+    fields:[FieldSpec];
+    // reserved_fields_map indicates how to index reserved fields to ingested fields
+    reserved_fields_map:TraceFieldMap;
     // duration determines how long a TraceSeries keeps its data
     duration:Duration;
-    // updated_at indicates when the TraceSeries is updated
-    updated_at:uint64;
+    // updated_nanoseconds_at indicates when the TraceSeries is updated
+    updated_nanoseconds_at:uint64;
 }
 
 // Catalog refers to a placement contains objects belonged to a particular data type
 enum Catalog:byte { Trace = 0, Log = 1, Metric = 2 }
 
 // IndexType determine the index structure under the hood
-enum IndexType:byte { Text = 0, Numerical = 1, ID = 2}
+// Fields with SeriesInternal type is reserved by Series module, would not be indexed by Index module.
+enum IndexType:byte { Text = 0, Numerical = 1, ID = 2, MultiText = 3, MultiNumberical = 4, SeriesInternal = 5 }
 
 // IndexObject defines who should be indexed.
 table IndexObject {

Review comment:
       > IndexObject is a portion of IndexRule instead of an individual resource. We don't set a group to it.
   > 
   > IndexRule has metadata that has a group.
   
   Yes, I understood. But according to the signature of `Repo.Search`, we have to pass a `indexName` (string) as the first argument, specifically, which name should be given 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r647940081



##########
File path: banyand/series/series.go
##########
@@ -22,29 +22,66 @@ import (
 	"context"
 
 	"github.com/apache/skywalking-banyandb/api/data"
+	v1 "github.com/apache/skywalking-banyandb/api/fbs/v1"
+	"github.com/apache/skywalking-banyandb/banyand/series/schema"
 	"github.com/apache/skywalking-banyandb/banyand/storage"
 	"github.com/apache/skywalking-banyandb/pkg/run"
 )
 
-type Trace interface {
+// TraceState represents the state of a trace link
+type TraceState int
+
+var (
+	TraceStateSuccess = 0
+	TraceStateError   = 1
+)
+
+//ScanOptions contain options
+//nolint
+type ScanOptions struct {
+	projection []string
+	state      TraceState
+	limit      uint32
+}
+
+//TraceRepo contains trace and entity data
+type TraceRepo interface {

Review comment:
       yep, we need it.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] lujiajing1126 commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r646247944



##########
File path: api/fbs/v1/schema.fbs
##########
@@ -27,21 +27,50 @@ struct Duration {
     unit:DurationUint;
 }
 
+enum FieldType:byte { String = 0, Int = 1, StringArray = 2, IntArray = 3 }
+
+table FieldSpec {
+    name:string;
+    type:FieldType;
+}
+
+table TraceStateMap {
+    field:string;
+    val_success:string;
+    val_error:string;
+}
+
+// The key in TraceFieldMap are reserved by trace series engine. Their corresponding value is the Fields or 
+// the combination of Fields
+table TraceFieldMap {
+    // trace_id the unique identity of a single trace
+    trace_id:string;
+    // series_id groups entites into a storage bucket
+    series_id:[string];
+    // state indicates the trace is "success" or "error"
+    state:TraceStateMap;
+}
+
 // TraceSeries represents a trace storage object
 table TraceSeries {
     // metadata is the identity of a trace series
     metadata:Metadata;
+    // fields defines accetped fields
+    fields:[FieldSpec];
+    // reserved_fields_map indicates how to index reserved fields to ingested fields
+    reserved_fields_map:TraceFieldMap;
     // duration determines how long a TraceSeries keeps its data
     duration:Duration;
-    // updated_at indicates when the TraceSeries is updated
-    updated_at:uint64;
+    // updated_nanoseconds_at indicates when the TraceSeries is updated

Review comment:
       `updated_at_nanoseconds`?

##########
File path: banyand/series/series.go
##########
@@ -22,29 +22,66 @@ import (
 	"context"
 
 	"github.com/apache/skywalking-banyandb/api/data"
+	v1 "github.com/apache/skywalking-banyandb/api/fbs/v1"
+	"github.com/apache/skywalking-banyandb/banyand/series/schema"
 	"github.com/apache/skywalking-banyandb/banyand/storage"
 	"github.com/apache/skywalking-banyandb/pkg/run"
 )
 
-type Trace interface {
+// TraceState represents the state of a trace link
+type TraceState int
+
+var (
+	TraceStateSuccess = 0
+	TraceStateError   = 1
+)
+
+//ScanOptions contain options
+//nolint
+type ScanOptions struct {
+	projection []string
+	state      TraceState
+	limit      uint32
+}
+
+//TraceRepo contains trace and entity data
+type TraceRepo interface {
+	//FetchTrace returns data.Trace by traceID
 	FetchTrace(traceID string) (data.Trace, error)
-	FetchEntity(chunkIDs []string, fields []string) ([]data.Entity, error)
-	ScanEntity(startTime, endTime uint64, fields []string) ([]data.Entity, error)
+	//FetchEntity returns data.Entity by ChunkID
+	FetchEntity(chunkIDs []string, opt ScanOptions) ([]data.Entity, error)

Review comment:
       `chunkIDs` should be `string` used here or `uint64` defined in `api/common/id.go`?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] lujiajing1126 commented on pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#issuecomment-857520949


   Shall we be worry about the performance hits caused by dereferencing the pointers to structs such as `Spec`, `Metadata`, etc.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r647939729



##########
File path: api/fbs/v1/schema.fbs
##########
@@ -27,21 +27,50 @@ struct Duration {
     unit:DurationUint;
 }
 
+enum FieldType:byte { String = 0, Int = 1, StringArray = 2, IntArray = 3 }
+
+table FieldSpec {
+    name:string;
+    type:FieldType;
+}
+
+table TraceStateMap {
+    field:string;
+    val_success:string;
+    val_error:string;
+}
+
+// The key in TraceFieldMap are reserved by trace series engine. Their corresponding value is the Fields or 
+// the combination of Fields
+table TraceFieldMap {
+    // trace_id the unique identity of a single trace
+    trace_id:string;
+    // series_id groups entites into a storage bucket
+    series_id:[string];
+    // state indicates the trace is "success" or "error"
+    state:TraceStateMap;
+}
+
 // TraceSeries represents a trace storage object
 table TraceSeries {
     // metadata is the identity of a trace series
     metadata:Metadata;
+    // fields defines accetped fields
+    fields:[FieldSpec];
+    // reserved_fields_map indicates how to index reserved fields to ingested fields
+    reserved_fields_map:TraceFieldMap;
     // duration determines how long a TraceSeries keeps its data
     duration:Duration;
-    // updated_at indicates when the TraceSeries is updated
-    updated_at:uint64;
+    // updated_nanoseconds_at indicates when the TraceSeries is updated

Review comment:
       make sense




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] lujiajing1126 commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r646309102



##########
File path: banyand/series/series.go
##########
@@ -22,29 +22,66 @@ import (
 	"context"
 
 	"github.com/apache/skywalking-banyandb/api/data"
+	v1 "github.com/apache/skywalking-banyandb/api/fbs/v1"
+	"github.com/apache/skywalking-banyandb/banyand/series/schema"
 	"github.com/apache/skywalking-banyandb/banyand/storage"
 	"github.com/apache/skywalking-banyandb/pkg/run"
 )
 
-type Trace interface {
+// TraceState represents the state of a trace link
+type TraceState int
+
+var (
+	TraceStateSuccess = 0
+	TraceStateError   = 1
+)
+
+//ScanOptions contain options
+//nolint
+type ScanOptions struct {
+	projection []string
+	state      TraceState
+	limit      uint32
+}
+
+//TraceRepo contains trace and entity data
+type TraceRepo interface {
+	//FetchTrace returns data.Trace by traceID
 	FetchTrace(traceID string) (data.Trace, error)
-	FetchEntity(chunkIDs []string, fields []string) ([]data.Entity, error)
-	ScanEntity(startTime, endTime uint64, fields []string) ([]data.Entity, error)
+	//FetchEntity returns data.Entity by ChunkID
+	FetchEntity(chunkIDs []string, opt ScanOptions) ([]data.Entity, error)
+	//ScanEntity returns data.Entity between a duration by ScanOptions
+	ScanEntity(startTime, endTime uint64, opt ScanOptions) ([]data.Entity, error)
 }
 
+//UniModel combines Trace, Metric and Log repositories into a union interface
 type UniModel interface {
-	Trace
+	TraceRepo
 }
 
+//SchemaRepo contains schema definition
+type SchemaRepo interface {
+	TraceSeries() schema.TraceSeries
+	IndexRule() schema.IndexRule
+	IndexRuleBinding() schema.IndexRuleBinding
+}
+
+//IndexFilter provides methods to find a specific index related objects
+type IndexFilter interface {
+	//RulesBySubject fetches IndexRule by Series defined in IndexRuleBinding
+	RulesBySubject(ctx context.Context, subject v1.Series) ([]v1.IndexRule, error)

Review comment:
       Add an API `FindRuleBySubjectAndField(s)` which returns a single IndexRule for the given field(s)




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-banyandb] hanahmily commented on a change in pull request #9: Add API for trace series, index rule and index rule binding

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-banyandb/pull/9#discussion_r647947071



##########
File path: api/fbs/v1/schema.fbs
##########
@@ -27,21 +27,50 @@ struct Duration {
     unit:DurationUint;
 }
 
+enum FieldType:byte { String = 0, Int = 1, StringArray = 2, IntArray = 3 }
+
+table FieldSpec {
+    name:string;
+    type:FieldType;
+}
+
+table TraceStateMap {
+    field:string;
+    val_success:string;
+    val_error:string;
+}
+
+// The key in TraceFieldMap are reserved by trace series engine. Their corresponding value is the Fields or 
+// the combination of Fields
+table TraceFieldMap {
+    // trace_id the unique identity of a single trace
+    trace_id:string;
+    // series_id groups entites into a storage bucket
+    series_id:[string];
+    // state indicates the trace is "success" or "error"
+    state:TraceStateMap;
+}
+
 // TraceSeries represents a trace storage object
 table TraceSeries {
     // metadata is the identity of a trace series
     metadata:Metadata;
+    // fields defines accetped fields
+    fields:[FieldSpec];
+    // reserved_fields_map indicates how to index reserved fields to ingested fields
+    reserved_fields_map:TraceFieldMap;
     // duration determines how long a TraceSeries keeps its data
     duration:Duration;
-    // updated_at indicates when the TraceSeries is updated
-    updated_at:uint64;
+    // updated_nanoseconds_at indicates when the TraceSeries is updated
+    updated_nanoseconds_at:uint64;
 }
 
 // Catalog refers to a placement contains objects belonged to a particular data type
 enum Catalog:byte { Trace = 0, Log = 1, Metric = 2 }
 
 // IndexType determine the index structure under the hood
-enum IndexType:byte { Text = 0, Numerical = 1, ID = 2}
+// Fields with SeriesInternal type is reserved by Series module, would not be indexed by Index module.
+enum IndexType:byte { Text = 0, Numerical = 1, ID = 2, MultiText = 3, MultiNumberical = 4, SeriesInternal = 5 }
 
 // IndexObject defines who should be indexed.
 table IndexObject {

Review comment:
       IndexObject is a portion of IndexRule instead of an individual resource. We don't set a group to it. 
   
   IndexRule has metadata that has a group. 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org