curvebs: support 512 aligned IO #1231

wu-hanqing · 2022-03-31T06:58:43Z

What problem does this PR solve?

Issue Number: close #985

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 · 2022-04-03T02:04:18Z

recheck

wu-hanqing · 2022-04-03T02:19:51Z

recheck

wu-hanqing · 2022-04-03T13:43:51Z

recheck

This reverts commit de2a121. Signed-off-by: wuhanqing <[email protected]>

wu-hanqing · 2022-06-24T02:00:06Z

recheck

wu-hanqing · 2022-06-24T09:29:33Z

recheck

ilixiaocui · 2022-07-12T08:55:28Z

conf/chunkserver.conf

+# it should consist with `block_size` in chunkfilepool.meta_path and `mds.volume.blockSize` in MDS's configurations
+# for clone chunk and snapshot chunk, it's also the minimum granularity that each bit represents
+# if set to |512|, we need 4096 bytes bitmap for each chunk, so meta_page_size should be 8192 or larger.
+global.block_size=4096


Will this value be verified on the mds side?

We have three verifications:

when a chunkserver starts, we will verify global.block_size in chunkserver's conf and chunkfilepool's metafile, if they aren't equal, chunkserver will quit.

then, if the chunkserver is newly deployed, and in the registration process, we will verify chunkserver's block size and block size recorded in MDS, if they aren't equal, chunkserver registration will fail.

MDS's conf also has a block_size, and when it starts, we will verify this item with block size recorded in etcd, if they aren't equal, mds will quit.

and for compatibility,

if chunkfilepool's metafile doesn't have block size, we treat it as 4096.

if etcd doesn't block size, we will insert it according to the value in conf.

ilixiaocui · 2022-07-18T03:08:31Z

curve-ansible/roles/generate_config/templates/s3.conf.j2

@@ -25,3 +25,4 @@ s3.throttle.iopsWriteLimit={{ s3_throttle_iopsWriteLimit }}
 s3.throttle.bpsTotalMB= {{ s3_throttle_bpsTotalLimit }}
 s3.throttle.bpsReadMB= {{ s3_throttle_bpsReadLimit }}
 s3.throttle.bpsWriteMB= {{ s3_throttle_bpsWriteLimit }}
+s3.useVirtualAddressing=False


What is this configuration for?

it was introduced by #1253

ilixiaocui · 2022-07-18T03:14:01Z

nbd/src/util.cpp

-    std::string path = "/sys/block/nbd" + std::to_string(nbd_index)
-                     + "/queue/hw_sector_size";
+    std::string path =
+        sys_block_path(nbd_index) + "/queue/logical_block_size";


Why did the check items change?

I'm not sure why we check hw_sector_size before, but what we set is block size when mapping the device,

https://github.com/opencurve/curve-nbd/blob/6231d3bc4954a5f2427148475cf39a2fdc4e3bad/src/NBDController.cpp#L50

ilixiaocui · 2022-07-18T03:32:46Z

src/chunkserver/config_info.h

@@ -140,6 +142,27 @@ struct ChunkServiceOptions {
    std::shared_ptr<InflightThrottle> inflightThrottle;
 };

+inline CopysetNodeOptions::CopysetNodeOptions()


This default configuration is only used when unit testing?

I didn't modify it, just moved it from source file to header, because it's trivial.

ilixiaocui · 2022-07-18T03:35:04Z

src/chunkserver/datastore/chunkserver_chunkfile.cpp

@@ -433,9 +438,9 @@ CSErrorCode CSChunkFile::Paste(const char * buf, off_t offset, size_t length) {

    // The request above must be pagesize aligned
    // the starting page index number of the paste area
-    uint32_t beginIndex = offset / pageSize_;
+    uint32_t beginIndex = offset / blockSize_;


Notes need to be changed.

// The request above must be pagesize aligned // the starting page index number of the paste area

ilixiaocui · 2022-07-18T03:35:52Z

src/chunkserver/datastore/chunkserver_chunkfile.cpp

@@ -488,9 +494,9 @@ CSErrorCode CSChunkFile::Read(char * buf, off_t offset, size_t length) {
    if (isCloneChunk_) {
        // The request above must be pagesize aligned
        // the starting page index number of the paste area
-        uint32_t beginIndex = offset / pageSize_;
+        uint32_t beginIndex = offset / blockSize_;


ilixiaocui · 2022-07-18T08:32:43Z

src/chunkserver/register.cpp

@@ -63,12 +63,14 @@ int Register::RegisterToMDS(ChunkServerMetadata *metadata) {
        req.set_externalip(ops_.chunkserverExternalIp);
    }
    req.set_port(ops_.chunkserverPort);
+    req.set_blocksize(ops_.blockSize);


Is this for verification on the mds side?

ilixiaocui · 2022-07-18T08:40:36Z

src/common/fast_align.h

@@ -0,0 +1,65 @@
+/*
+Copyright 2015 Glen Joseph Fernandes
+([email protected])


Same as curvefs/src/common/fast_align.h?

fix, curvefs/src/common/fast_align.h is removed

ilixiaocui · 2022-07-18T08:41:28Z

src/common/namespace_define.h

@@ -62,6 +62,8 @@ const char CLONEINFOKEYEND[] = "13";
 const char DISCARDSEGMENTKEYPREFIX[] = "13";
 const char DISCARDSEGMENTKEYEND[] = "14";

+const char BLOCKSIZEKEY[] = "14blocksize";


Why not use "15blocksize" ?

ilixiaocui · 2022-07-18T08:50:49Z

src/tools/curve_format_main.cpp

+    }
+
+    auto bitmapBytes = FLAGS_fileSize / FLAGS_blockSize / 8;
+    if (bitmapBytes > FLAGS_metaPageSize * 2) {


If bitmapBytes > (FLAGS_metaPageSize-specified field), it should not work?
Here means if bitmapBytes = 4k, FLAGS_metaPageSize = 1k，that's ok？

fixed, I added a hard code value 4096 - 512 to indicate minimum size usage for encoding and storing source location for clone chunk, so that, the meta page size should be great than or equal to 3584 + bitmap bytes

Signed-off-by: Hanqing Wu <[email protected]>

ilixiaocui · 2022-08-01T03:04:34Z

curve-ansible/server.ini

@@ -101,6 +101,8 @@ chunk_size=16777216
 chunkserver_walfilepool_segment_size=8388608
 retain_pool=False
 walfilepool_use_chunk_file_pool=True
+chunkserver_meta_page_size=4096


Consider chunkserver_meta_page_size, which is automatically calculated by chunkserver according to the chunkserver_block_size?

xu-chaojie · 2022-08-03T07:12:15Z

src/mds/server/mds.cpp

@@ -500,6 +507,12 @@ void MDS::InitCurveFSOptions(CurveFSOption *curveFSOptions) {
        "mds.curvefs.minFileLength", &curveFSOptions->minFileLength);
    conf_->GetValueFatalIfFail(
        "mds.curvefs.maxFileLength", &curveFSOptions->maxFileLength);
+    conf_->GetValueFatalIfFail("mds.curvefs.blockSize", &g_block_size);


why not define g_block_size in curveFSOptions instead of global variable

xu-chaojie · 2022-08-03T07:21:39Z

src/mds/topology/topology_service_manager.cpp

 void TopologyServiceManager::RegistChunkServer(
    const ChunkServerRegistRequest *request,
    ChunkServerRegistResponse *response) {
+    if (request->has_blocksize()) {


Consider that chunkserver gets blocksize from mds instead of configuring blocksize individually and then verifying

xu-chaojie · 2022-08-03T07:25:37Z

src/tools/curve_format_main.cpp

+
+DEFINE_validator(metaPageSize, &ValidateMetaPageSize);
+
+DEFINE_uint32(blockSize, 4096, "minimum io alignment supported");


Do we check blockSize and metaPageSize at chunkserver startup ?

ilixiaocui · 2022-12-07T02:33:40Z

cc @wu-hanqing Please resubmit to the master branch if necessary.

wu-hanqing force-pushed the bs/512-aligned-io branch 7 times, most recently from 2932059 to 09cef26 Compare April 3, 2022 01:55

wu-hanqing force-pushed the bs/512-aligned-io branch 3 times, most recently from 726becd to a7fe2e2 Compare April 3, 2022 11:45

wu-hanqing force-pushed the bs/512-aligned-io branch from a7fe2e2 to a5b44d7 Compare June 23, 2022 09:19

Revert "client/chunkserver/curve-nbd: support 512 aligned IO requests"

b8cccd2

This reverts commit de2a121. Signed-off-by: wuhanqing <[email protected]>

wu-hanqing force-pushed the bs/512-aligned-io branch 2 times, most recently from 94f1947 to b668629 Compare June 23, 2022 12:36

wu-hanqing force-pushed the bs/512-aligned-io branch from b668629 to 2836175 Compare June 24, 2022 06:48

ilixiaocui reviewed Jul 12, 2022

View reviewed changes

ilixiaocui reviewed Jul 18, 2022

View reviewed changes

wu-hanqing force-pushed the bs/512-aligned-io branch from 2836175 to 078e7ab Compare July 26, 2022 07:52

curvebs: support 512 aligned IO

89ca60c

Signed-off-by: Hanqing Wu <[email protected]>

wu-hanqing force-pushed the bs/512-aligned-io branch from 078e7ab to 89ca60c Compare July 26, 2022 09:45

ilixiaocui reviewed Aug 2, 2022

View reviewed changes

xu-chaojie reviewed Aug 3, 2022

View reviewed changes

wu-hanqing mentioned this pull request Aug 6, 2022

get block size from backend storage opencurve/curve-nbd#1

Closed

ilixiaocui closed this Dec 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

curvebs: support 512 aligned IO #1231

curvebs: support 512 aligned IO #1231

wu-hanqing commented Mar 31, 2022 •

edited

Loading

wu-hanqing commented Apr 3, 2022

wu-hanqing commented Apr 3, 2022

wu-hanqing commented Apr 3, 2022

wu-hanqing commented Jun 24, 2022

wu-hanqing commented Jun 24, 2022

ilixiaocui Jul 12, 2022

wu-hanqing Jul 21, 2022

ilixiaocui Jul 18, 2022

wu-hanqing Jul 21, 2022

ilixiaocui Jul 18, 2022

wu-hanqing Jul 26, 2022

ilixiaocui Jul 18, 2022

wu-hanqing Jul 21, 2022

ilixiaocui Jul 18, 2022

wu-hanqing Jul 21, 2022

ilixiaocui Jul 18, 2022

wu-hanqing Jul 21, 2022

ilixiaocui Jul 18, 2022

wu-hanqing Jul 21, 2022

ilixiaocui Jul 18, 2022

wu-hanqing Jul 21, 2022

ilixiaocui Jul 18, 2022

wu-hanqing Jul 21, 2022

ilixiaocui Jul 18, 2022

wu-hanqing Jul 26, 2022

ilixiaocui Aug 1, 2022

xu-chaojie Aug 3, 2022

xu-chaojie Aug 3, 2022

xu-chaojie Aug 3, 2022 •

edited

Loading

ilixiaocui commented Dec 7, 2022


		DEFINE_validator(metaPageSize, &ValidateMetaPageSize);

		DEFINE_uint32(blockSize, 4096, "minimum io alignment supported");

curvebs: support 512 aligned IO #1231

curvebs: support 512 aligned IO #1231

Conversation

wu-hanqing commented Mar 31, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

wu-hanqing commented Apr 3, 2022

wu-hanqing commented Apr 3, 2022

wu-hanqing commented Apr 3, 2022

wu-hanqing commented Jun 24, 2022

wu-hanqing commented Jun 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xu-chaojie Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

ilixiaocui commented Dec 7, 2022

wu-hanqing commented Mar 31, 2022 •

edited

Loading

xu-chaojie Aug 3, 2022 •

edited

Loading