Optimize backend impl to not hold open files

This patch changes the filesystem-backend implementation of libimagstore
to open files on each read/write rather than holding the file handle in
memory at all times.

Whenever a lot of imag store entries are read into memory, the imag
process may ran out of file descriptors. With this patch applied, a
`Store::get()` call on an entry which is not yet in the store cache
would cause the file to be read, but the FD being dropped after that.
Likewise, a `Store::update()` (which is also called if the imag entry is
dropped) would re-open the file on the filesystem and write the contents
from the imag store cache back to the file.

With this patch, opening hundrets or thousands of imag entries should be
no problem anymore, only the available memory should be a limit then.

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
This commit is contained in:
Matthias Beyer 2018-08-07 02:38:22 +02:00
parent f2916aa51a
commit 8b508fe4c3
1 changed files with 18 additions and 51 deletions

View File

@ -32,10 +32,7 @@ use storeid::StoreId;
use file_abstraction::iter::PathIterator; use file_abstraction::iter::PathIterator;
#[derive(Debug)] #[derive(Debug)]
pub enum FSFileAbstractionInstance { pub struct FSFileAbstractionInstance(PathBuf);
Absent(PathBuf),
File(File, PathBuf)
}
impl FileAbstractionInstance for FSFileAbstractionInstance { impl FileAbstractionInstance for FSFileAbstractionInstance {
@ -44,32 +41,18 @@ impl FileAbstractionInstance for FSFileAbstractionInstance {
*/ */
fn get_file_content(&mut self, id: StoreId) -> Result<Entry, SE> { fn get_file_content(&mut self, id: StoreId) -> Result<Entry, SE> {
debug!("Getting lazy file: {:?}", self); debug!("Getting lazy file: {:?}", self);
let (file, path) = match *self {
FSFileAbstractionInstance::File(ref mut f, _) => return {
// We seek to the beginning of the file since we expect each
// access to the file to be in a different context
f.seek(SeekFrom::Start(0))
.chain_err(|| SEK::FileNotSeeked)?;
let mut s = String::new(); let mut file = open_file(&self.0)
f.read_to_string(&mut s) .chain_err(|| SEK::FileNotFound)?;
.chain_err(|| SEK::IoError)
.map(|_| s) file.seek(SeekFrom::Start(0)).chain_err(|| SEK::FileNotSeeked)?;
.and_then(|s| Entry::from_str(id, &s))
}, let mut s = String::new();
FSFileAbstractionInstance::Absent(ref p) =>
(open_file(p).chain_err(|| SEK::FileNotFound)?, p.clone()), file.read_to_string(&mut s)
}; .chain_err(|| SEK::IoError)
*self = FSFileAbstractionInstance::File(file, path); .map(|_| s)
if let FSFileAbstractionInstance::File(ref mut f, _) = *self { .and_then(|s| Entry::from_str(id, &s))
let mut s = String::new();
f.read_to_string(&mut s)
.chain_err(|| SEK::IoError)
.map(|_| s)
.and_then(|s| Entry::from_str(id, &s))
} else {
unreachable!()
}
} }
/** /**
@ -78,28 +61,12 @@ impl FileAbstractionInstance for FSFileAbstractionInstance {
fn write_file_content(&mut self, buf: &Entry) -> Result<(), SE> { fn write_file_content(&mut self, buf: &Entry) -> Result<(), SE> {
use std::io::Write; use std::io::Write;
let buf = buf.to_str()?.into_bytes(); let buf = buf.to_str()?.into_bytes();
let mut file = create_file(&self.0).chain_err(|| SEK::FileNotCreated)?;
let (file, path) = match *self { file.seek(SeekFrom::Start(0)).chain_err(|| SEK::FileNotCreated)?;
FSFileAbstractionInstance::File(ref mut f, _) => return { file.set_len(buf.len() as u64).chain_err(|| SEK::FileNotWritten)?;
// We seek to the beginning of the file since we expect each file.write_all(&buf).chain_err(|| SEK::FileNotWritten)
// access to the file to be in a different context
f.seek(SeekFrom::Start(0))
.chain_err(|| SEK::FileNotCreated)?;
f.set_len(buf.len() as u64).chain_err(|| SEK::FileNotWritten)?;
f.write_all(&buf).chain_err(|| SEK::FileNotWritten)
},
FSFileAbstractionInstance::Absent(ref p) =>
(create_file(p).chain_err(|| SEK::FileNotCreated)?, p.clone()),
};
*self = FSFileAbstractionInstance::File(file, path);
if let FSFileAbstractionInstance::File(ref mut f, _) = *self {
trace!("Writing buffer...");
return f.write_all(&buf).chain_err(|| SEK::FileNotWritten);
}
unreachable!();
} }
} }
@ -149,7 +116,7 @@ impl FileAbstraction for FSFileAbstraction {
} }
fn new_instance(&self, p: PathBuf) -> Box<FileAbstractionInstance> { fn new_instance(&self, p: PathBuf) -> Box<FileAbstractionInstance> {
Box::new(FSFileAbstractionInstance::Absent(p)) Box::new(FSFileAbstractionInstance(p))
} }
/// We return nothing from the FS here. /// We return nothing from the FS here.