You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@skywalking.apache.org by "ButterBright (via GitHub)" <gi...@apache.org> on 2025/11/25 15:11:58 UTC

[PR] Define new bulk write api [skywalking-banyandb]

ButterBright opened a new pull request, #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863

   <!--
       ⚠️ Please make sure to read this template first, pull requests that don't accord with this template
       maybe closed without notice.
       Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
       Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
   -->
   
   <!-- ==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👇 ====
   ### Fix <bug description or the bug issue number or bug issue link>
   - [ ] Add a unit test to verify that the fix works.
   - [ ] Explain briefly why the bug exists and how to fix it.
        ==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👆 ==== -->
   
   <!-- ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👇 ====
   ### <Feature description>
   - [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
   - [ ] Update the documentation to include this new feature.
   - [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
   - [ ] If it's UI related, attach the screenshots below.
        ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👆 ==== -->
   
   - [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Fixes apache/skywalking#13480.
   - [ ] Update the [`CHANGES` log](https://github.com/apache/skywalking-banyandb/blob/main/CHANGES.md).
   


-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2564781200


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -42,11 +58,13 @@ message DataPointValue {
 // WriteRequest is the request contract for write
 message WriteRequest {
   // the metadata is required.
-  common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];
+  common.v1.Metadata metadata = 1;
   // the data_point is required.
   DataPointValue data_point = 2 [(validate.rules).message.required = true];
   // the message_id is required.
   uint64 message_id = 3 [(validate.rules).uint64.gt = 0];
+  // the data point specification.
+  DataPointSpec data_point_spec = 4;

Review Comment:
   ```suggestion
     // the data point specification. 
     // If this is not set with the indicated metadata, use the schema definitions.
     // If this is not set, use the existing spec declaration from previous write requests in the current gRPC stream.
     DataPointSpec data_point_spec = 4;
   ```



##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -42,11 +58,13 @@ message DataPointValue {
 // WriteRequest is the request contract for write
 message WriteRequest {
   // the metadata is required.
-  common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];
+  common.v1.Metadata metadata = 1;

Review Comment:
   // Metadata is only required for the first write request of the gRPC stream



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2562035468


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   @hanahmily told me, you two wanted to use compatible mode when the first row of bulk doesn't have spec info. Then, it should be fine.
   
   There are 2 cases now
   - Expected new style
   ```
   -- In Bulk A
   spec: tag1, tag2, tag3, field 1, field2
   data1: data point value1
   data2: data point value2
   data3: data point value3
   spec2: tag1, tag3, tag4, field 2, field5
   data3: data point value4
   ```
   
   - Historic and compatible style
   ```
   -- In Bulk B
   #no spec field in row1
   data1: data point value1
   data2: data point value2
   data3: data point value3
   data3: data point value4
   ```



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2560465886


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   >otherwise match the measure schema
   
   We should not take this. For new implementation, let's break the compatibility. And bump up client API version to 0.10
   https://skywalking.apache.org/docs/skywalking-banyandb/next/installation/versions/



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2562035468


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   @hanahmily told me, you two wanted to use compatible mode when the first row of bulk doesn't have spec info. Then, it should be fine.
   But note, we will have different spec(s) in one bulk, like the above example mentioned. 
   
   There are three cases now
   - Expected new style
   ```
   -- In Bulk A
   spec: tag1, tag2, tag3, field 1, field2
   data1: data point value1
   data2: data point value2
   data3: data point value3
   spec2: tag1, tag3, tag4, field 2, field5
   data3: data point value4
   ```
   
   - Historic and compatible style
   ```
   -- In Bulk B
   #no spec field in row1
   data1: data point value1
   data2: data point value2
   data3: data point value3
   data3: data point value4
   ```
   
   - An unusual style
   ```
   -- In Bulk C
   #no spec field in row1
   data1: data point value1
   data2: data point value2
   data3: data point value3
   spec2: tag1, tag3, tag4, field 2, field5
   data3: data point value4
   ```



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2560465886


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   >otherwise match the measure schema
   
   We should not take this. For new implementation, let's break the compatibility. And bump up client API version to 0.10
   https://skywalking.apache.org/docs/skywalking-banyandb/next/installation/versions/



##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   @hanahmily Do you want to keep backward compatibility? So we could skip data_point_spec?
   
   The tricky point is, if you want to improve performance, we only should set `data_point_spec` for the first row of a part of bulk, e.g.
   
   ```
   -- In Bulk A
   spec: tag1, tag2, tag3, field 1, field2
   data1: data point value1
   data2: data point value2
   data3: data point value3
   spec2: tag1, tag3, tag4, field 2, field5
   data3: data point value4



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2562035468


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   @hanahmily told me, you two wanted to use compatible mode when the first row of bulk doesn't have spec info. Then, it should be fine.
   But note, we will have different spec(s) in one bulk, like the above example mentioned. 
   
   There are 2 cases now
   - Expected new style
   ```
   -- In Bulk A
   spec: tag1, tag2, tag3, field 1, field2
   data1: data point value1
   data2: data point value2
   data3: data point value3
   spec2: tag1, tag3, tag4, field 2, field5
   data3: data point value4
   ```
   
   - Historic and compatible style
   ```
   -- In Bulk B
   #no spec field in row1
   data1: data point value1
   data2: data point value2
   data3: data point value3
   data3: data point value4
   ```



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2564761663


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -42,11 +58,13 @@ message DataPointValue {
 // WriteRequest is the request contract for write
 message WriteRequest {
   // the metadata is required.
-  common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];
+  common.v1.Metadata metadata = 1;

Review Comment:
   // Metadata is not required for the first write request of the gRPC stream



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.

Copilot commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2560387921


##########
api/proto/banyandb/trace/v1/write.proto:
##########
@@ -26,11 +26,16 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/trace/v1";
 option java_package = "org.apache.skywalking.banyandb.trace.v1";
 
+message TagSpec {

Review Comment:
   The `TagSpec` message is missing documentation comments. Similar messages in other proto files (e.g., `TagFamilySpec` in stream/v1/write.proto and measure/v1/write.proto) include descriptive comments explaining the message purpose and its fields. Consider adding comments like:
   ```
   // TagSpec defines the specification of tags.
   message TagSpec {
     // names of tags
     repeated string tag_names = 1;
   }
   ```
   ```suggestion
   
   // TagSpec defines the specification of tags.
   message TagSpec {
     // names of tags
   ```



##########
api/proto/banyandb/stream/v1/write.proto:
##########
@@ -27,14 +27,22 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/stream/v1";
 option java_package = "org.apache.skywalking.banyandb.stream.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}

Review Comment:
   The `TagFamilySpec` message is duplicated between stream/v1/write.proto and measure/v1/write.proto with identical structure. Consider defining this message once in a common location (e.g., banyandb/model/v1/write.proto) and importing it in both stream and measure to avoid duplication and ensure consistency. This improves maintainability as any future changes to TagFamilySpec would only need to be made in one place.



##########
api/proto/banyandb/trace/v1/write.proto:
##########
@@ -26,11 +26,16 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/trace/v1";
 option java_package = "org.apache.skywalking.banyandb.trace.v1";
 
+message TagSpec {
+  repeated string tag_names = 1;
+}
+
 message WriteRequest {
   common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];
   repeated model.v1.TagValue tags = 2 [(validate.rules).repeated.min_items = 1];
   bytes span = 3;
   uint64 version = 4 [(validate.rules).uint64.gt = 0];

Review Comment:
   The `tag_spec` field is missing a documentation comment. Other similar fields in stream/v1/write.proto and measure/v1/write.proto have descriptive comments. Consider adding a comment like:
   ```
     // the tag specification.
     TagSpec tag_spec = 5;
   ```
   ```suggestion
     uint64 version = 4 [(validate.rules).uint64.gt = 0];
     // the tag specification.
   ```



##########
api/proto/banyandb/trace/v1/write.proto:
##########
@@ -26,11 +26,16 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/trace/v1";
 option java_package = "org.apache.skywalking.banyandb.trace.v1";
 
+message TagSpec {
+  repeated string tag_names = 1;
+}
+
 message WriteRequest {
   common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];

Review Comment:
   The `tags` field should have a comment clarifying the ordering behavior when `tag_spec` is provided, similar to how it's documented in stream/v1/write.proto and measure/v1/write.proto. Consider adding a comment like:
   ```
     // the order of tags' items match TagSpec if provided, otherwise match the trace schema
     repeated model.v1.TagValue tags = 2 [(validate.rules).repeated.min_items = 1];
   ```
   ```suggestion
     common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];
     // the order of tags' items match TagSpec if provided, otherwise match the trace schema
   ```



##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}

Review Comment:
   The `TagFamilySpec` message is duplicated between stream/v1/write.proto and measure/v1/write.proto with identical structure. Consider defining this message once in a common location (e.g., banyandb/model/v1/write.proto) and importing it in both stream and measure to avoid duplication and ensure consistency. This improves maintainability as any future changes to TagFamilySpec would only need to be made in one place.



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2560470472


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   This metadata style is widely used in the gRPC ecosystem, e.g., Envoy ALS. And in practice, it would not increase the bandwidth cost too much as in large scale, each format will carry a large data scale, even the whole bulk.
   The overview performance is super close to the index-based write.



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2562035468


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   @hanahmily told me, you two wanted to use compatible mode when the first row of bulk doesn't have spec info. Then, it should be fine.
   
   There are 2 cases now
   - Expected new style
   ```
   -- In Bulk A
   spec: tag1, tag2, tag3, field 1, field2
   data1: data point value1
   data2: data point value2
   data3: data point value3
   data3: data point value4
   ```
   
   - Historic and compatible style
   ```
   -- In Bulk B
   #no spec field in row1
   data1: data point value1
   data2: data point value2
   data3: data point value3
   data3: data point value4
   ```



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2560470472


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   This metadata style is widely used in the gRPC ecosystem, e.g. Envoy ALS



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2564756532


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema
   repeated model.v1.TagFamilyForWrite tag_families = 2 [(validate.rules).repeated.min_items = 1];
-  // the order of fields match the measure schema
+  // the order of fields match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   ```suggestion
     // the order of fields match DataPointSpec
   ```



##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema
   repeated model.v1.TagFamilyForWrite tag_families = 2 [(validate.rules).repeated.min_items = 1];
-  // the order of fields match the measure schema
+  // the order of fields match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   This comment is not proper anymore



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2560470472


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   This metadata style is widely used in the gRPC ecosystem, e.g., Envoy ALS. And in practice, it would not increase the bandwidth cost too much as in large scale, each format will carry a large data scale, even the whole bulk.
   The overview performance is super close to the index-based write.



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on code in PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#discussion_r2560460061


##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -27,13 +27,29 @@ import "validate/validate.proto";
 option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1";
 option java_package = "org.apache.skywalking.banyandb.measure.v1";
 
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+  // name of the tag family
+  string name = 1;
+  // names of tags in the tag family
+  repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+  // the tag family specification
+  repeated TagFamilySpec tag_family_spec = 1;
+  // the field names
+  repeated string field_names = 2;
+}
+
 // DataPointValue is the data point for writing. It only contains values.
 message DataPointValue {
   // timestamp is in the timeunit of milliseconds.
   google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required = true];
-  // the order of tag_families' items match the measure schema
+  // the order of tag_families' items match DataPointSpec if provided, otherwise match the measure schema

Review Comment:
   @hanahmily Do you want to keep backward compatibility? So we could skip data_point_spec?
   
   The tricky point is, if you want to improve performance, we only should set `data_point_spec` for the first row of a part of bulk, e.g.
   
   ```
   -- In Bulk A
   spec: tag1, tag2, tag3, field 1, field2
   data1: data point value1
   data2: data point value2
   data3: data point value3
   spec2: tag1, tag3, tag4, field 2, field5
   data3: data point value4



-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.

codecov-commenter commented on PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#issuecomment-3576218348

   ## [Codecov](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/863?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 32.17%. Comparing base ([`3530dd9`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/3530dd9b5cefaa4fef60eec91ecbfbdfcaf243c5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`497d6ed`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/497d6ed52de22c1afa288114a849d6441e1f078d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 42 commits behind head on main.
   > :exclamation:  There is a different number of reports uploaded between BASE (3530dd9) and HEAD (497d6ed). Click for more details.
   > 
   > <details><summary>HEAD has 1 upload less than BASE</summary>
   >
   >| Flag | BASE (3530dd9) | HEAD (497d6ed) |
   >|------|------|------|
   >||1|0|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##             main     #863       +/-   ##
   ===========================================
   - Coverage   45.97%   32.17%   -13.81%     
   ===========================================
     Files         328       95      -233     
     Lines       55505     9855    -45650     
   ===========================================
   - Hits        25520     3171    -22349     
   + Misses      27909     6379    -21530     
   + Partials     2076      305     -1771     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/863/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [pkg](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/863/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `32.17% <ø> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/863?dropdown=coverage&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).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#issuecomment-3581065336

   Please update the comments with a proper description. I put some for missing points.


-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.

wu-sheng commented on PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863#issuecomment-3581294494

   Please fix CI.


-- 
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] Define new bulk write api [skywalking-banyandb]

Posted by "ButterBright (via GitHub)" <gi...@apache.org>.

ButterBright merged PR #863:
URL: https://github.com/apache/skywalking-banyandb/pull/863


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