Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

curvebs: support 512 aligned IO #2538

Merged
merged 2 commits into from
Jul 3, 2023
Merged

Conversation

wu-hanqing
Copy link
Contributor

What problem does this PR solve?

Issue Number: #xxx

Problem Summary:

What is changed and how it works?

What's Changed:

How it Works:

Side effects(Breaking backward compatibility? Performance regression?):

Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing wu-hanqing force-pushed the pr-1231-2 branch 2 times, most recently from 9a3df88 to e848709 Compare June 19, 2023 11:27
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

1 similar comment
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

return os;
}

// bool CheckFilePoolMetaWithOptions(const FilePoolOptions& options,
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix

fi->blocksize = finfo.blocksize();
} else {
// for backward compatibility
fi->blocksize = 4096;
Copy link
Contributor

Choose a reason for hiding this comment

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

fi->blocksize = kDefaultBlockSize?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix

@wu-hanqing
Copy link
Contributor Author

cicheck

1 similar comment
@wu-hanqing
Copy link
Contributor Author

cicheck

// chunkfile.
// Currently, we need to ensure the atomicity of the meta page update, so set
// its size to 4k.
constexpr size_t kChunkfileMetaPageSize = 4096;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should metaPageSize change when block size is 512?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

meta page size is fixed, and only a tiny portion of the meta page is used to store bitmap so that we will reduce the chunk size.

nebd::client::GetInfoResponse response;

request.set_fd(fd);
stub.GetInfo(cntl, &request, &response, nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Get from part2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep

@@ -114,6 +114,7 @@ class ChunkServiceImpl : public ChunkService {
uint32_t maxChunkSize_;

std::shared_ptr<EpochMap> epochMap_;
uint32_t align_;
Copy link
Contributor

Choose a reason for hiding this comment

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

blockSize_?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix

This reverts commit de2a121.

Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

4 similar comments
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

2 similar comments
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
@wu-hanqing
Copy link
Contributor Author

cicheck

1 similar comment
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing wu-hanqing merged commit 73defb0 into opencurve:master Jul 3, 2023
3 checks passed
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.

None yet

3 participants