Skip to content

Conversation

@cassyunknown
Copy link

@cassyunknown cassyunknown commented Feb 11, 2022

Adds support for recovering from file-based datapool, intended for use with PMEM.

@CLAassistant
Copy link

CLAassistant commented Feb 11, 2022

CLA assistant check
All committers have signed the CLA.

self.datapool_path.as_ref().map(|v| Path::new(v).to_owned())
}

pub fn segments_fields_path(&self) -> Option<PathBuf> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should each of these paths be it's own config option? I suspect we can be opinionated about the names for each file if we decide to keep the parts separate.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I was going to wait until we decided how many files to use

/// Copies `size` bytes at `byte_ptr` to the `offset` of `data`
/// Returns the next `offset`, that is, the next byte of `data` to be copied into
pub fn store_bytes_and_update_offset(byte_ptr: *const u8, offset: usize, size: usize, data: &mut [u8]) -> usize {
// get corresponding bytes from byte pointer
Copy link
Author

Choose a reason for hiding this comment

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

I put this function here so that Segments, TtlBuckets and HashTable could all access it. I am slightly concerned about security as I am not sure if this could be used as a "write what I want where I want"


if config.graceful_shutdown() {
// TODO: check if successfully shutdown and record result
self.data.flush();
Copy link
Author

Choose a reason for hiding this comment

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

This line currently produces a warning as I am ignoring the Result. Ideally the result would be intepreted here or passed along to server/segcache/src/lib.rs where I intend this flush() function to be called


/// Flush (gracefully shutdown) the `Seg` cache if configured to do so
pub fn flush<T: SegConfig>(self, config: &T) {
let config = config.seg();
Copy link
Author

@cassyunknown cassyunknown Feb 17, 2022

Choose a reason for hiding this comment

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

I couldn't find a trait that I should implement (that only has the flush() required method) so I just put this function as a method of Seg. I did the same thing for the other files I added a flush() to

… not point in keeping this until there is a nice way to determine expected file size
…ll change code so that Segments use same file. Also need to tidy code up
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