curvebs/chunkserver: optimize sync chunk when write in bulk #1912

fansehep · 2022-09-10T13:02:34Z

What problem does this PR solve?

Issue Number: #1887

Problem Summary: see issue.

What is changed and how it works?

What's Changed:
I use SyncChunkThread instread of the synctimer, Let each write determine on its own whether to notify. and the sumWrite is shared. Default 30s or sum of write >= 4 MB(maybe 4MB is not better) will force sync.
We need some tests or benchmarks for this changes to verify this design is better than old synctimer design.

And i think 4 MB as the default threshold is not accurate, Maybe we can use binary search to find the best thredhold for most machines.

fansehep · 2022-09-13T03:25:53Z

recheck

fansehep · 2022-09-13T04:46:30Z

recheck

fansehep · 2022-09-14T00:03:22Z

recheck

fansehep · 2022-09-16T12:35:01Z

recheck

conf/chunkserver.conf

src/chunkserver/copyset_node.cpp

fansehep · 2022-11-23T11:17:57Z

recheck

fansehep · 2022-11-24T10:35:41Z

recheck

wu-hanqing · 2022-11-24T10:42:16Z

recheck

ilixiaocui · 2022-11-24T12:43:23Z

recheck

cw123 · 2022-11-28T09:08:02Z

recheck

src/chunkserver/chunkserver.cpp

ilixiaocui · 2022-11-29T13:59:29Z

recheck

src/chunkserver/copyset_node.cpp

ilixiaocui · 2022-11-30T10:49:01Z

combined into one commit.

fansehep · 2022-11-30T11:06:42Z

combined into one commit.

should review it firstly？

ilixiaocui · 2022-11-30T10:50:30Z

conf/chunkserver.conf

@@ -88,6 +88,8 @@ copyset.raft_snapshot_uri=curve://./0/copysets  # __CURVEADM_TEMPLATE__ curve://
 copyset.recycler_uri=local://./0/recycler  # __CURVEADM_TEMPLATE__ local://${prefix}/data/recycler __CURVEADM_TEMPLATE__
 # chunkserver启动时，copyset并发加载的阈值,为0则表示不做限制
 copyset.load_concurrency=10
+# chunkserver 使用多少个线程来完成copysetsync, 不能为0


please use english comment.

ilixiaocui · 2022-11-30T10:50:57Z

src/chunkserver/config_info.h

@@ -111,6 +111,10 @@ struct CopysetNodeOptions {

    // 限制chunkserver启动时copyset并发恢复加载的数量,为0表示不限制
    uint32_t loadConcurrency = 0;
+    // chunkserver sync_thread_pool 的线程数量


ilixiaocui · 2022-11-30T10:54:10Z

src/chunkserver/config_info.h

+    // syncChunkLimit default limit
+    uint64_t syncChunkLimit = 2 * 1024 * 1024;
+    // syncHighChunkLimit default limit = 64k
+    uint64_t syncThreshold = 65536;


It should be better to use 64 * 1024 instead of 65536.

ilixiaocui · 2022-11-30T10:56:28Z

src/chunkserver/copyset_node.cpp

@@ -54,6 +56,8 @@ using curve::fs::FileSystemInfo;

 const char *kCurveConfEpochFilename = "conf.epoch";

+uint32_t CopysetNode::syncTriggerSeconds_ = 25;


Why use global variables？

ilixiaocui · 2022-11-30T11:00:46Z

src/chunkserver/copyset_node.h

+    /**
+     * each copyset use copyset_pool to sync
+     */
+    void setSharedSyncPool(const std::shared_ptr<TaskThreadPool<>>& threadpool);


Why not initialize syncTriggerSeconds_ in CopysetNode::Init

ilixiaocui · 2022-11-30T11:01:53Z

src/chunkserver/copyset_node_manager.h

@@ -223,6 +223,8 @@ class CopysetNodeManager : public curve::common::Uncopyable {
    CopysetNodeOptions copysetNodeOptions_;
    // 控制copyset并发启动的数量
    std::shared_ptr<TaskThreadPool<>> copysetLoader_;
+    // copysetmap 的sync操作全部通过一个线程池来完成


Please use English comments for new code.

ilixiaocui · 2022-11-30T11:11:08Z

src/chunkserver/datastore/chunkserver_chunkfile.cpp

+        *chunkrate_ += length;
+        uint64_t res = *chunkrate_;
+        auto actualSyncChunkLimits = syncChunkLimits_;
+        // if single write size > syncThreshold, for cache friend to


It is best to change the rate at the entry of CSChunkFile::Write. And update of syncChunkLimits_ can be put into a function like MayUpdateLimits(uint64_t writelen)

ilixiaocui · 2022-11-30T11:15:19Z

src/chunkserver/datastore/chunkserver_datastore.h

 private:
+    uint64_t syncChunkLimits_;
+    uint64_t syncThreshold_;


Why are these two variables syncChunkLimits_ and syncThreshold_ contained in three classes CSMetaCache，CSChunkFile and CopysetNode?

fansehep · 2022-12-01T03:33:37Z

recheck

wu-hanqing · 2022-12-02T02:26:47Z

@fansehep you can find whether https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=428b1d6b29ca599c5700d4bc4f4ce4c5880369bf is useful in our case, according to the description the logical is almost same with ours.

fansehep · 2022-12-02T02:48:09Z

@fansehep you can find whether https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=428b1d6b29ca599c5700d4bc4f4ce4c5880369bf is useful in our case, according to the description the logical is almost same with ours.

Ok. thanks

Signed-off-by: fan <yfan3763@gmail.com>

ilixiaocui · 2022-12-02T05:40:29Z

recheck

fansehep · 2022-12-07T05:43:52Z

recheck

fansehep closed this Sep 12, 2022

fansehep force-pushed the opm_flush_log branch from 3feeb37 to bab48e0 Compare September 12, 2022 09:27

fansehep reopened this Sep 12, 2022

fansehep force-pushed the opm_flush_log branch 3 times, most recently from d37de94 to 0a60829 Compare September 13, 2022 16:47

fansehep force-pushed the opm_flush_log branch 2 times, most recently from 262c718 to 8e7ac71 Compare September 16, 2022 13:03

legionxiong reviewed Sep 29, 2022

View reviewed changes

conf/chunkserver.conf Outdated Show resolved Hide resolved

fansehep force-pushed the opm_flush_log branch 2 times, most recently from d57cffc to 724fdc4 Compare September 29, 2022 05:00

legionxiong reviewed Oct 10, 2022

View reviewed changes

src/chunkserver/copyset_node.cpp Outdated Show resolved Hide resolved

fansehep force-pushed the opm_flush_log branch from 8ede4d2 to 3e70dfc Compare October 10, 2022 13:01

fansehep force-pushed the opm_flush_log branch from 3e70dfc to 1947d22 Compare October 19, 2022 03:51

fansehep changed the base branch from master1 to master October 19, 2022 03:52

fansehep force-pushed the opm_flush_log branch from 1947d22 to 7b08a0a Compare November 23, 2022 09:47

fansehep force-pushed the opm_flush_log branch 2 times, most recently from 34dd3e2 to a1b842f Compare November 24, 2022 08:44

fansehep force-pushed the opm_flush_log branch from a1b842f to 9e43ac1 Compare November 24, 2022 11:22

fansehep force-pushed the opm_flush_log branch from 9e43ac1 to c6301a0 Compare November 24, 2022 13:55

legionxiong approved these changes Nov 29, 2022

View reviewed changes

src/chunkserver/chunkserver.cpp Show resolved Hide resolved

fansehep force-pushed the opm_flush_log branch from c6301a0 to 63ece62 Compare November 29, 2022 03:48

legionxiong reviewed Nov 30, 2022

View reviewed changes

src/chunkserver/copyset_node.cpp Outdated Show resolved Hide resolved

fansehep force-pushed the opm_flush_log branch from 63ece62 to 1837ffd Compare November 30, 2022 05:26

ilixiaocui reviewed Nov 30, 2022

View reviewed changes

fansehep force-pushed the opm_flush_log branch 2 times, most recently from bb58d5c to e2c3cb8 Compare December 1, 2022 02:48

fansehep force-pushed the opm_flush_log branch 2 times, most recently from a0b4982 to 7a015d4 Compare December 1, 2022 07:10

fansehep force-pushed the opm_flush_log branch from 7a015d4 to 8b10c41 Compare December 2, 2022 04:17

sync by threadpool

559a4b1

Signed-off-by: fan <yfan3763@gmail.com>

fansehep force-pushed the opm_flush_log branch from 8b10c41 to 559a4b1 Compare December 2, 2022 04:24

ilixiaocui approved these changes Dec 7, 2022

View reviewed changes

ilixiaocui merged commit 11b88a8 into opencurve:master Dec 7, 2022

zhanghuidinah mentioned this pull request Dec 7, 2022

[2022 Q4 Developer Activities]: Call For Participation！ #2017

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

curvebs/chunkserver: optimize sync chunk when write in bulk #1912

curvebs/chunkserver: optimize sync chunk when write in bulk #1912

fansehep commented Sep 10, 2022 •

edited

Loading

fansehep commented Sep 13, 2022

fansehep commented Sep 13, 2022

fansehep commented Sep 14, 2022

fansehep commented Sep 16, 2022

fansehep commented Nov 23, 2022

fansehep commented Nov 24, 2022

wu-hanqing commented Nov 24, 2022

ilixiaocui commented Nov 24, 2022

cw123 commented Nov 28, 2022

ilixiaocui commented Nov 29, 2022

ilixiaocui commented Nov 30, 2022

fansehep commented Nov 30, 2022

ilixiaocui Nov 30, 2022

ilixiaocui Nov 30, 2022

ilixiaocui Nov 30, 2022

ilixiaocui Nov 30, 2022

ilixiaocui Nov 30, 2022

ilixiaocui Nov 30, 2022

ilixiaocui Nov 30, 2022

ilixiaocui Nov 30, 2022

fansehep commented Dec 1, 2022

wu-hanqing commented Dec 2, 2022

fansehep commented Dec 2, 2022 •

edited

Loading

ilixiaocui commented Dec 2, 2022

fansehep commented Dec 7, 2022

		@@ -54,6 +56,8 @@ using curve::fs::FileSystemInfo;

		const char *kCurveConfEpochFilename = "conf.epoch";

		uint32_t CopysetNode::syncTriggerSeconds_ = 25;

curvebs/chunkserver: optimize sync chunk when write in bulk #1912

curvebs/chunkserver: optimize sync chunk when write in bulk #1912

Conversation

fansehep commented Sep 10, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

fansehep commented Sep 13, 2022

fansehep commented Sep 13, 2022

fansehep commented Sep 14, 2022

fansehep commented Sep 16, 2022

fansehep commented Nov 23, 2022

fansehep commented Nov 24, 2022

wu-hanqing commented Nov 24, 2022

ilixiaocui commented Nov 24, 2022

cw123 commented Nov 28, 2022

ilixiaocui commented Nov 29, 2022

ilixiaocui commented Nov 30, 2022

fansehep commented Nov 30, 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

fansehep commented Dec 1, 2022

wu-hanqing commented Dec 2, 2022

fansehep commented Dec 2, 2022 • edited Loading

ilixiaocui commented Dec 2, 2022

fansehep commented Dec 7, 2022

fansehep commented Sep 10, 2022 •

edited

Loading

fansehep commented Dec 2, 2022 •

edited

Loading