Skip to content

Conversation

@jessicayu626
Copy link
Contributor

No description provided.

@jessicayu626 jessicayu626 requested a review from Antzen November 27, 2017 05:40
Copy link
Contributor

@Antzen Antzen left a comment

Choose a reason for hiding this comment

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

Overall, looks fine to me, but of course we should run some additional end-to-end and unit tests to ensure correctness.

if (!zk->create(delete_item, block_vec, error_code, false)) {
LOG(INFO) << "[recover_file] pushed create " << delete_item;
}
blockDeleted(block_id, dn);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good to me, but we should double check that this is a valid delete process with some additional unit tests.

memcpy(&block_data, &block_data_vec[0], sizeof(block_data));
uint64_t block_size = block_data.block_size;

if (size_left < block_size) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this case would ever be executed under our current implementation of file appends, but it's probably good to keep if future implementations would use "reserve blocks" for file mutability.

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