Skip to content

Conversation

@hanahmily
Copy link
Contributor

@hanahmily hanahmily added this to the 0.10.0 milestone Dec 23, 2025
@hanahmily hanahmily added the bug Something isn't working label Dec 23, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical panic in the measure block merger that occurred when merging blocks with overlapping timestamps. The issue manifested as an "index out of range [-1]" error when the merger attempted to access an array element at index -1.

Key Changes:

  • Added bounds check in mergeTwoBlocks to prevent array index out of bounds panic
  • Added optimization to swap blocks upfront when the right block starts with a smaller timestamp
  • Comprehensive test coverage including edge case tests and file-based merge scenarios

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
banyand/measure/merger.go Fixed panic by adding bounds check (i > left.idx) before accessing left.timestamps[i-1] and added optimization to swap blocks if right starts with smaller timestamp
banyand/measure/merger_test.go Added Test_mergeTwoBlocks_edgeCase to test the specific panic scenario, added Test_mergeParts_fileBased with 4 comprehensive sub-tests, and added helper functions for generating test data
CHANGES.md Added bug fix entry describing the panic fix for measure block merger

The changes are well-implemented with proper edge case handling and comprehensive test coverage. The fix correctly addresses the root cause by ensuring array bounds are validated before access, and the optimization helps reduce unnecessary swaps during the merge process. The test cases thoroughly cover various scenarios including overlapping timestamps, block splitting when exceeding maxBlockLength, and multiple blocks per part.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.11%. Comparing base (3530dd9) to head (dca2e96).
⚠️ Report is 83 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #914      +/-   ##
==========================================
+ Coverage   45.97%   46.11%   +0.14%     
==========================================
  Files         328      368      +40     
  Lines       55505    56572    +1067     
==========================================
+ Hits        25520    26091     +571     
- Misses      27909    28015     +106     
- Partials     2076     2466     +390     
Flag Coverage Δ
banyand 49.29% <100.00%> (?)
bydbctl 81.91% <ø> (?)
fodc 90.65% <ø> (?)
integration-distributed 80.00% <ø> (?)
pkg 29.07% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hanahmily hanahmily merged commit dbfcebc into apache:main Dec 23, 2025
21 checks passed
@hanahmily hanahmily deleted the bug/measure-merge branch December 23, 2025 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Banyandb standlone mode query data error then db can't start

4 participants