[pbs-devel] [PATCH v3 proxmox-backup 1/5] fix #3847: datastore: support writing fidx files of unknown size
Robert Obkircher
r.obkircher at proxmox.com
Fri Jan 9 18:35:36 CET 2026
Use mremap and ftruncate to support growable FixedIndexWriters. Grow
exponentially from a small initial index size for efficiency. Truncate
excessive capacity after encountering a non-full block or on close.
Signed-off-by: Robert Obkircher <r.obkircher at proxmox.com>
---
pbs-datastore/src/datastore.rs | 2 +-
pbs-datastore/src/fixed_index.rs | 120 +++++++++++++++++++++++++++++--
2 files changed, 117 insertions(+), 5 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 9c57aaac..af712726 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -591,7 +591,7 @@ impl DataStore {
pub fn create_fixed_writer<P: AsRef<Path>>(
&self,
filename: P,
- size: usize,
+ size: Option<usize>,
chunk_size: usize,
) -> Result<FixedIndexWriter, Error> {
let index = FixedIndexWriter::create(
diff --git a/pbs-datastore/src/fixed_index.rs b/pbs-datastore/src/fixed_index.rs
index 6c3be2d4..8036a519 100644
--- a/pbs-datastore/src/fixed_index.rs
+++ b/pbs-datastore/src/fixed_index.rs
@@ -219,9 +219,12 @@ pub struct FixedIndexWriter {
chunk_size: usize,
size: usize,
index_length: usize,
+ index_capacity: usize,
index: *mut u8,
pub uuid: [u8; 16],
pub ctime: i64,
+ growable_size: bool,
+ write_size_on_close: bool,
}
// `index` is mmap()ed which cannot be thread-local so should be sendable
@@ -237,12 +240,18 @@ impl Drop for FixedIndexWriter {
}
impl FixedIndexWriter {
+ /// The initial capacity, if the total size is unknown.
+ ///
+ /// This capacity takes up the same amount of space as the header
+ /// and can refer to 128 Blocks * 4 MiB/Block = 512 MiB of content.
+ const INITIAL_CAPACITY: usize = 4096 / 32;
+
#[allow(clippy::cast_ptr_alignment)]
// Requires obtaining a shared chunk store lock beforehand
pub fn create(
store: Arc<ChunkStore>,
path: &Path,
- size: usize,
+ known_size: Option<usize>,
chunk_size: usize,
) -> Result<Self, Error> {
let full_path = store.relative_path(path);
@@ -264,6 +273,7 @@ impl FixedIndexWriter {
}
let ctime = proxmox_time::epoch_i64();
+ let size = known_size.unwrap_or(0);
let uuid = Uuid::generate();
@@ -280,8 +290,12 @@ impl FixedIndexWriter {
file.write_all(&buffer)?;
- let index_length = size.div_ceil(chunk_size);
- let index_size = index_length * 32;
+ let (index_length, index_capacity) = known_size
+ .map(|s| s.div_ceil(chunk_size))
+ .map(|len| (len, len))
+ .unwrap_or((0, Self::INITIAL_CAPACITY));
+
+ let index_size = index_capacity * 32;
nix::unistd::ftruncate(&file, (header_size + index_size) as i64)?;
let data = unsafe {
@@ -305,12 +319,87 @@ impl FixedIndexWriter {
chunk_size,
size,
index_length,
+ index_capacity,
index: data,
ctime,
uuid: *uuid.as_bytes(),
+ growable_size: known_size.is_none(),
+ write_size_on_close: known_size.is_none(),
+ })
+ }
+
+ /// If this returns an error, the sizes may be out of sync,
+ /// which is especially bad if the capacity was reduced.
+ fn set_index_capacity(&mut self, new_capacity: usize) -> Result<(), Error> {
+ if new_capacity == self.index_capacity {
+ return Ok(());
+ }
+ let old_index_size = self.index_capacity * 32;
+ let new_index_size = new_capacity * 32;
+ let new_file_size = (size_of::<FixedIndexHeader>() + new_index_size) as i64;
+
+ let index_addr = NonNull::new(self.index as *mut std::ffi::c_void).ok_or_else(|| {
+ format_err!("Can't resize FixedIndexWriter index because the index pointer is null.")
+ })?;
+
+ nix::unistd::ftruncate(&self.file, new_file_size)?;
+
+ let new_index = unsafe {
+ nix::sys::mman::mremap(
+ index_addr,
+ old_index_size,
+ new_index_size,
+ nix::sys::mman::MRemapFlags::MREMAP_MAYMOVE,
+ None,
+ )
+ }?;
+
+ self.index = new_index.as_ptr().cast::<u8>();
+ self.index_capacity = new_capacity;
+ Ok(())
+ }
+
+ /// Unmapping ensures future add and close operations fail.
+ fn set_index_capacity_or_unmap(&mut self, new_capacity: usize) -> Result<(), Error> {
+ self.set_index_capacity(new_capacity).map_err(|e| {
+ let unmap_result = self.unmap();
+ let message = format!(
+ "failed to resize index capacity from {} to {new_capacity} with backing file: {:?}",
+ self.index_capacity, self.tmp_filename
+ );
+ assert!(self.index.is_null(), "{message} {unmap_result:?}");
+ e.context(message)
})
}
+ /// Increase the content size to be at least `requested_size` and
+ /// ensure there is enough capacity.
+ ///
+ /// Only writers that were created without a known size can grow.
+ /// The size also becomes fixed as soon as it is no longer divisible
+ /// by the block size, to ensure that only the last block can be
+ /// smaller.
+ pub fn grow_to_size(&mut self, requested_size: usize) -> Result<(), Error> {
+ if self.size < requested_size {
+ if !self.growable_size {
+ bail!("refusing to resize from {} to {requested_size}", self.size);
+ }
+ let new_len = requested_size.div_ceil(self.chunk_size);
+ if new_len * self.chunk_size != requested_size {
+ // not a full chunk, so this must be the last one
+ self.growable_size = false;
+ self.set_index_capacity_or_unmap(new_len)?;
+ } else if new_len > self.index_capacity {
+ self.set_index_capacity_or_unmap(new_len.next_power_of_two())?;
+ };
+ assert!(new_len <= self.index_capacity);
+ self.index_length = new_len;
+ self.size = requested_size;
+ }
+ Ok(())
+ }
+
+ /// The current length of the index. This may be increased with [`grow_to_size`].
pub fn index_length(&self) -> usize {
self.index_length
}
@@ -320,7 +409,7 @@ impl FixedIndexWriter {
return Ok(());
};
- let index_size = self.index_length * 32;
+ let index_size = self.index_capacity * 32;
if let Err(err) = unsafe { nix::sys::mman::munmap(index, index_size) } {
bail!("unmap file {:?} failed - {}", self.tmp_filename, err);
@@ -342,9 +431,24 @@ impl FixedIndexWriter {
self.unmap()?;
+ if self.index_length == 0 {
+ bail!("refusing to close empty fidx file {:?}", self.tmp_filename);
+ } else if self.index_length < self.index_capacity {
+ let file_size = size_of::<FixedIndexHeader>() + index_size;
+ nix::unistd::ftruncate(&self.file, file_size as i64)?;
+ self.index_capacity = self.index_length;
+ }
+
let csum_offset = std::mem::offset_of!(FixedIndexHeader, index_csum);
self.file.seek(SeekFrom::Start(csum_offset as u64))?;
self.file.write_all(&index_csum)?;
+
+ if self.write_size_on_close {
+ let size_offset = std::mem::offset_of!(FixedIndexHeader, size);
+ self.file.seek(SeekFrom::Start(size_offset as u64))?;
+ self.file.write_all(&(self.size as u64).to_le_bytes())?;
+ }
+
self.file.flush()?;
if let Err(err) = std::fs::rename(&self.tmp_filename, &self.filename) {
@@ -407,6 +511,14 @@ impl FixedIndexWriter {
}
pub fn clone_data_from(&mut self, reader: &FixedIndexReader) -> Result<(), Error> {
+ if self.growable_size {
+ bail!("reusing the index is only supported with known input size");
+ }
+
+ if self.chunk_size != reader.chunk_size {
+ bail!("can't reuse file with different chunk size");
+ }
+
if self.index_length != reader.index_count() {
bail!("clone_data_from failed - index sizes not equal");
}
--
2.47.3
More information about the pbs-devel
mailing list