Skip to content

[ENH]: Add ability to set different block sizes for different blockfiles #4948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sanketkedia
Copy link
Contributor

@sanketkedia sanketkedia commented Jun 26, 2025

Description of changes

Summarize the changes made by this PR.

  • Improvements & Bug fixes
    • We persist the max_block_size_bytes in the arrow metadata now and also add a new blockfile version for handling backward compatibility with old data
    • This is need so that spann posting list can have a block size different from other blockfiles
    • This PR also sets the spann PL block size to 5MiB
  • New functionality
    • ...

Test plan

How are these changes tested?

  • Tests pass locally with pytest for python, yarn test for js, cargo test for rust

Documentation Changes

None

Copy link
Contributor Author

sanketkedia commented Jun 26, 2025

Copy link

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

  • Can you think of any use case in which the code does not behave as intended? Have they been tested?
  • Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
  • If appropriate, are there adequate property based tests?
  • If appropriate, are there adequate unit tests?
  • Should any logging, debugging, tracing information be added or removed?
  • Are error messages user-friendly?
  • Have all documentation changes needed been made?
  • Have all non-obvious changes been commented?

System Compatibility

  • Are there any potential impacts on other parts of the system or backward compatibility?
  • Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

  • Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

@sanketkedia sanketkedia requested a review from codetheweb June 26, 2025 01:49
@sanketkedia sanketkedia marked this pull request as ready for review June 26, 2025 01:49
Copy link
Contributor

propel-code-bot bot commented Jun 26, 2025

Add Per-Blockfile Block Size Setting with Versioning and Backward Compatibility

This PR introduces the ability to specify and persist different block sizes for distinct blockfiles in the Arrow blockstore. It updates the Arrow metadata schema to persist the max_block_size_bytes per-blockfile, adds a new V1.2 blockfile version for backward compatibility, and sets Spann posting list block size to 5MiB. The necessary migration logic is added to upgrade older blockfiles, and the blockfile provider APIs, writer options, and internal handling are updated to support and test this functionality. Additional temporary debug logs regarding IO task limits and scheduling are also present.

Key Changes:
• Blockfile writers and readers can now set and use a per-blockfile max_block_size_bytes, persisted in Arrow schema metadata starting with version V1.2.
• Arrow root and blockfile structs, creation logic, and migrations updated to support the new block size metadata and ensure backward compatibility for V1 and V1.1.
• Blockfile provider, writer options, and underlying APIs extended to accept per-writer block size overrides.
• Spann index sets its posting list block size to 5MiB via new config path.
• Extensive test updates/additions for V1.2 support, metadata round-trip, forking, and compatibility with legacy files.
• Temporary println! and debug logs added for tracing scheduler and IO task behavior.

Affected Areas:
• rust/blockstore/src/arrow/root.rs
• rust/blockstore/src/arrow/blockfile.rs
• rust/blockstore/src/arrow/ordered_blockfile_writer.rs
• rust/blockstore/src/arrow/provider.rs
• rust/blockstore/src/arrow/migrations.rs
• rust/blockstore/src/types/writer_options.rs
• rust/index/src/spann/types.rs
• tests in blockstore and spann

This summary was automatically generated by @propel-code-bot

@@ -39,6 +39,8 @@ use crate::{

use super::utils::{rng_query, KMeansAlgorithmInput, KMeansError, RngQueryError};

const PL_BLOCK_SIZE: usize = 5 * 1024 * 1024; // 5 MiB
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should add a justification for why this

@sanketkedia sanketkedia force-pushed the 06-19-_enh_granular_lock_in_spann_index branch from f99a00a to ab8839e Compare June 26, 2025 07:36
@sanketkedia sanketkedia force-pushed the 06-25-_enh_add_ability_to_set_different_block_sizes_for_different_blockfiles branch from 56c1e29 to 3c09498 Compare June 26, 2025 07:36
}
Ok(())
}

async fn migrate_v1_to_v1_1(
Copy link
Contributor

Choose a reason for hiding this comment

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

should update this method name

@sanketkedia sanketkedia force-pushed the 06-19-_enh_granular_lock_in_spann_index branch 2 times, most recently from c13149b to 561c851 Compare June 26, 2025 17:04
@sanketkedia sanketkedia force-pushed the 06-25-_enh_add_ability_to_set_different_block_sizes_for_different_blockfiles branch from 3c09498 to adfa3f8 Compare June 26, 2025 17:04
@sanketkedia sanketkedia force-pushed the 06-19-_enh_granular_lock_in_spann_index branch from 561c851 to 9f05434 Compare June 26, 2025 18:01
@sanketkedia sanketkedia force-pushed the 06-25-_enh_add_ability_to_set_different_block_sizes_for_different_blockfiles branch from adfa3f8 to 27cdd92 Compare June 26, 2025 18:01
@sanketkedia sanketkedia force-pushed the 06-19-_enh_granular_lock_in_spann_index branch from 9f05434 to 28e3c9e Compare June 26, 2025 18:24
@sanketkedia sanketkedia force-pushed the 06-25-_enh_add_ability_to_set_different_block_sizes_for_different_blockfiles branch from 27cdd92 to 1b00d59 Compare June 26, 2025 18:24
@sanketkedia sanketkedia changed the base branch from 06-19-_enh_granular_lock_in_spann_index to graphite-base/4948 June 26, 2025 19:22
@sanketkedia sanketkedia force-pushed the 06-25-_enh_add_ability_to_set_different_block_sizes_for_different_blockfiles branch from 1b00d59 to 0b3c66e Compare June 26, 2025 19:23
@graphite-app graphite-app bot changed the base branch from graphite-base/4948 to main June 26, 2025 19:23
@sanketkedia sanketkedia force-pushed the 06-25-_enh_add_ability_to_set_different_block_sizes_for_different_blockfiles branch 2 times, most recently from c3b23af to f9ead28 Compare June 26, 2025 21:26
@sanketkedia sanketkedia force-pushed the 06-25-_enh_add_ability_to_set_different_block_sizes_for_different_blockfiles branch from f9ead28 to 2293092 Compare June 27, 2025 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants