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 2026/06/27 13:52:37 UTC
[PR] Fix block metadata reset before unmarshal for stream and trace [skywalking-banyandb]
ButterBright opened a new pull request, #1195:
URL: https://github.com/apache/skywalking-banyandb/pull/1195
<!--
β οΈ 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 π ==== -->
- [x] 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] Fix block metadata reset before unmarshal for stream and trace [skywalking-banyandb]
Posted by "ButterBright (via GitHub)" <gi...@apache.org>.
ButterBright merged PR #1195:
URL: https://github.com/apache/skywalking-banyandb/pull/1195
--
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] Fix block metadata reset before unmarshal for stream and trace [skywalking-banyandb]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1195:
URL: https://github.com/apache/skywalking-banyandb/pull/1195#discussion_r3489462212
##########
banyand/stream/block_metadata_test.go:
##########
@@ -246,6 +246,7 @@ func Test_unmarshalBlockMetadata(t *testing.T) {
original := []blockMetadata{
{
seriesID: common.SeriesID(1),
+ tagType: make(tagType),
timestamps: timestampsMetadata{
Review Comment:
These expectations were updated to account for per-entry reset during unmarshalling, but the test still only unmarshals into a nil destination slice. That wonβt catch the regression this PR fixes (reusing a preallocated dst via dst[:0] can retain stale tagFamilies/tagType without resetting the reused element). Add a subtest that pre-populates a dst slice (cap > 0) with a blockMetadata containing non-empty maps, then call unmarshalBlockMetadata(dst[:0], marshaled) and assert the decoded entries donβt include the stale keys.
##########
banyand/trace/block_metadata_test.go:
##########
@@ -207,6 +207,7 @@ func Test_unmarshalBlockMetadata(t *testing.T) {
wanted := []blockMetadata{
{
traceID: "trace1",
+ tags: make(map[string]*dataBlock),
tagType: make(map[string]pbv1.ValueType),
spans: &dataBlock{
offset: 10,
Review Comment:
The expected results were adjusted for reset-in-unmarshal behavior, but this test still unmarshals into a nil destination slice. The bug fix is about reusing a destination slice (dst[:0]) where underlying entries may still hold stale tags/tagType maps. Add a subtest that pre-populates a dst slice (cap > 0) with a blockMetadata containing non-empty tags/tagType, then call unmarshalBlockMetadata(dst[:0], marshaled, tagType) and assert the decoded result doesnβt contain the stale keys.
--
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] Fix block metadata reset before unmarshal for stream and trace [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily commented on PR #1195:
URL: https://github.com/apache/skywalking-banyandb/pull/1195#issuecomment-4829244514
@ButterBright, fix the comments from Copilot.
--
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] Fix block metadata reset before unmarshal for stream and trace [skywalking-banyandb]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1195:
URL: https://github.com/apache/skywalking-banyandb/pull/1195#issuecomment-4818261740
## [Codecov](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1195?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 42.57%. 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 ([`6c0d01a`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/6c0d01aaacd81aea8bb22995893927be4af70a7c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 312 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #1195 +/- ##
==========================================
- Coverage 45.97% 42.57% -3.41%
==========================================
Files 328 215 -113
Lines 55505 21056 -34449
==========================================
- Hits 25520 8965 -16555
+ Misses 27909 11195 -16714
+ Partials 2076 896 -1180
```
| [Flag](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1195/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/1195/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `42.57% <ΓΈ> (?)` | |
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 Harness](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1195?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