Copilot commented on code in PR #804:
URL:
https://github.com/apache/skywalking-banyandb/pull/804#discussion_r2422560782
##########
banyand/trace/tstable_test.go:
##########
@@ -305,3 +306,64 @@ func generateHugeTraces(num int) *traces {
traces.spanIDs = append(traces.spanIDs, []string{"span2", "span3"}...)
return traces
}
+
+// generateRealisticTraces creates a more realistic dataset for benchmarking:
+// - Each trace has 3-5 spans
+// - Each span is more than 100KB
+// - Creates the specified number of unique traces.
+func generateRealisticTraces(numTraces int) *traces {
+ traces := &traces{
+ traceIDs: []string{},
+ timestamps: []int64{},
+ tags: [][]*tagValue{},
+ spans: [][]byte{},
+ spanIDs: []string{},
+ }
+
+ // Create a large span payload (>100KB)
+ // Using a mix of realistic data: stack traces, error messages, metadata
+ spanPayloadTemplate := make([]byte, 110*1024) // 110KB
Review Comment:
[nitpick] The comment states '110KB' but this creates a slice with exactly
110*1024 bytes. Consider using more descriptive units like '110 * 1024' or
adding a constant for clarity.
```suggestion
// Create a large span payload (>100 KiB)
// Using a mix of realistic data: stack traces, error messages, metadata
const spanPayloadSizeKiB = 110 // 110 KiB (1 KiB = 1024 bytes)
spanPayloadTemplate := make([]byte, spanPayloadSizeKiB*1024)
```
##########
banyand/trace/merger.go:
##########
@@ -365,8 +378,20 @@ func mergeBlocks(closeCh <-chan struct{}, bw *blockWriter,
br *blockReader) (*pa
bw.mustWriteBlock(pendingBlock.bm.traceID,
&pendingBlock.block)
releaseDecoder()
pendingBlock.reset()
+ pendingBlockIsEmpty = true
Review Comment:
This assignment is redundant since `pendingBlockIsEmpty` is already set to
`true` at line 365 after `pendingBlock.reset()`. The variable state is already
correct at this point.
```suggestion
```
##########
banyand/trace/block_reader.go:
##########
@@ -29,6 +29,13 @@ import (
"github.com/apache/skywalking-banyandb/pkg/pool"
)
+type rawBlock struct {
+ bm *blockMetadata
+ tags map[string][]byte
+ tagMetadata map[string][]byte
+ spans []byte
+}
Review Comment:
The `rawBlock` struct lacks documentation. Add a comment explaining its
purpose for raw block data transfer without unmarshaling overhead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
