Changes for full store ranges for object store metadata reads

[bharath-techie]

Description

[Describe what this change achieves]

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Global State Race set_whole_region_fetch_enabled is called without synchronization in create_reader, which can be invoked concurrently. If two threads call create_reader simultaneously with different store_ptr values (one zero, one non-zero), the global WHOLE_REGION_FETCH_ENABLED flag may be set inconsistently, causing one reader to use the wrong fetch strategy. This can lead to cache misses or incorrect behavior when mixing local and remote stores. crate::cache::page_index::set_whole_region_fetch_enabled(store_ptr > 0);

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Global flag causes concurrency issues Setting a global flag based on store_ptr > 0 in create_reader can cause race conditions if multiple readers are created concurrently with different store types. The flag affects all subsequent operations globally, not just the current reader. sandbox/plugins/analytics-backend-datafusion/rust/src/api.rs [797] +// Consider passing store type as a parameter to fetch functions instead of +// using a global flag, or use thread-local storage to avoid cross-reader interference crate::cache::page_index::set_whole_region_fetch_enabled(store_ptr > 0); Suggestion importance[1-10]: 8 __ Why: This identifies a significant concurrency issue where multiple concurrent create_reader calls with different store types could interfere with each other through the global WHOLE_REGION_FETCH_ENABLED flag. The flag set by one reader affects all subsequent fetch operations globally, potentially causing incorrect fetch strategies for other readers.	Medium
	Use stronger memory ordering Using Ordering::Relaxed for a global flag that controls fetch strategy across threads can lead to visibility issues. Consider using Ordering::SeqCst or Ordering::Release/Acquire pairs to ensure the flag change is visible to all threads immediately. sandbox/plugins/analytics-backend-datafusion/rust/src/cache/page_index/mod.rs [113-123] pub(crate) static WHOLE_REGION_FETCH_ENABLED: AtomicBool = AtomicBool::new(false); ... pub fn set_whole_region_fetch_enabled(enabled: bool) { - WHOLE_REGION_FETCH_ENABLED.store(enabled, Ordering::Relaxed); + WHOLE_REGION_FETCH_ENABLED.store(enabled, Ordering::SeqCst); } Suggestion importance[1-10]: 6 __ Why: While Ordering::Relaxed may cause visibility delays across threads, the flag is set once during create_reader initialization before concurrent operations begin. However, using Ordering::SeqCst would provide stronger guarantees and is safer for a global configuration flag that affects fetch strategy.	Low
Possible issue	Validate buffer count before access The buffers.first()? call will return None if the vector is empty, but get_ranges with a single range should always return exactly one buffer. Add an explicit length check before accessing to make the invariant clear and prevent silent failures. sandbox/plugins/analytics-backend-datafusion/rust/src/cache/page_index/page_index_io.rs [504-505] let buffers = store.get_ranges(location, std::slice::from_ref(&region)).await.ok()?; -return Some(IndexBuffers::Shared { base: region.start, bytes: buffers.first()?.clone() }); +if buffers.len() != 1 { + return None; +} +return Some(IndexBuffers::Shared { base: region.start, bytes: buffers[0].clone() }); Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that get_ranges with a single range should return exactly one buffer. Adding an explicit length check makes the invariant clearer and prevents potential silent failures if the implementation changes. The buffers.first()? pattern could mask unexpected empty results.	Medium

[github-actions]

❌ Gradle check result for c11dfaa: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?