client/mds: merge stripe feature and unit test #211

baijiaruo · 2021-01-05T11:46:03Z

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:

What is changed and how it works?

What's Changed: add stripe feature

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

bai-charisu · 2021-01-14T09:40:29Z

curvefs_python/curvefs.py

-#  See the License for the specific language governing permissions and
-#  limitations under the License.
-#
-


为什么把license删掉了？

这个文件重新生成的时候默认没有，已补充

bai-charisu · 2021-01-14T09:43:39Z

curvefs_python/curvefs_wrap.cxx

- *  See the License for the specific language governing permissions and
- *  limitations under the License.
- */
-


为什么去掉了license？

bai-charisu · 2021-01-14T10:51:14Z

curvefs_python/curvefs_wrap.cxx

+      if (val) *val = static_cast< unsigned int >(v);
+    }
+  }
+  return res;


这一段是做什么的

这个是swig -c++ -python curvefs.i自动生成的

bai-charisu · 2021-01-14T11:17:32Z

src/mds/nameserver2/curvefs.cpp

+    }
+
+    if (stripeUnit > defaultChunkSize_) {
+        LOG(ERROR) << "stripeUnit more then chunksize.stripeUnit:"


bai-charisu · 2021-01-14T11:20:23Z

src/mds/nameserver2/curvefs.cpp

+
+    return StatusCode::kOK;
+}
+


stripeCount是不是不能大于chunksize/stripeSize？

stripeCount理论上只需要满足defaultChunkSize_ % stripeCount = 0就可以了，但是实际使用上太大没有意义，不做修改

bai-charisu · 2021-01-15T06:12:55Z

src/client/splitor.cpp

+            uint64_t curChunkSetIndex = stripeIndex / stripesPerChunk;
+            uint64_t curChunkIndex = curChunkSetIndex * stripeCount + stripepos;
+
+            uint64_t blockInChunkStart = (stripeIndex % stripesPerChunk)


这个应该叫blockStartOff？

这个表示的是某个block在chunk的起始位置，即start位置，已改成blockInChunkStartOff

bai-charisu · 2021-01-15T06:18:40Z

test/client/iotracker_splitor_unittest.cpp

+
+    FInfo_t fi;
+    uint64_t offset = 1 * 1024 * 1024 - 64 * 1024;
+    uint64_t length = 128 * 1024;


增加一个跨chunkset的用例吧

wu-hanqing · 2021-01-05T12:13:41Z

curvefs_python/curvefs.py

@@ -1,19 +1,3 @@
-#


这个版权也加回去吧

wu-hanqing · 2021-01-05T12:17:40Z

include/client/libcurve.h

+ * @param: filename文件名
+ * @param: userinfo是当前打开或创建时携带的user信息
+ * @param: size文件长度，当create为true的时候以size长度创建文件
+ * @param:


这里stripe的两个参数注释没加

新增的部分都使用英文注释

wu-hanqing · 2021-01-05T12:19:03Z

include/client/libcurve.h

+ */
+int Create2(const char* filename,
+           const C_UserInfo_t* userinfo,
+           size_t size, uint64_t stripeUnit, uint64_t stripeCount);


这两个用uint32_t就行了吧

考虑扩展性，可不改

wu-hanqing · 2021-01-05T12:21:49Z

src/client/mds_client.cpp

@@ -376,6 +378,8 @@ LIBCURVE_ERROR MDSClient::GetFileInfo(const std::string& filename,

        if (response.has_fileinfo()) {
            FileInfo finfo = response.fileinfo();
+            LOG(INFO) << "stripeUnit:" << finfo.stripeunit()


这个日志可以去掉吧？

wu-hanqing · 2021-01-11T11:46:53Z

curvefs_python/curvefs_wrap.cxx

@@ -5695,8 +5806,8 @@ SWIGINTERN PyObject *_wrap_GetClusterId(PyObject *SWIGUNUSEDPARM(self), PyObject
  PyObject * obj0 = 0 ;
  PyObject * obj1 = 0 ;
  int result;
-  int retlen;
-
+   int retlen;


wu-hanqing · 2021-01-12T03:17:22Z

test/integration/client/mds_exception_test.cpp

+                testIORead = true;
+            }
+            std::unique_lock<std::mutex> lk(resumeMtx);
+            resumeCV.notify_all();


这里没有delete context

wu-hanqing · 2021-01-12T03:20:23Z

test/integration/client/mds_exception_test.cpp

+
+        readThread.join();
+        ASSERT_TRUE(testIORead);
+        ASSERT_EQ(strcmp(writebuf, readbuf), 0);


strcmp依赖末尾的\0，这里可能会出问题。用strncmp吧

writebuf的内容是不会出现\0的，只会是26个字母

wu-hanqing · 2021-01-12T03:24:23Z

test/integration/client/mds_exception_test.cpp

+     *  @return: 数据是否一致
+    */
+    void VerifyDataConsistency(int fd, uint64_t offset, uint64_t size) {
+        char* writebuf = new char[size];


这两个buffer没有delete

wu-hanqing · 2021-01-18T01:50:45Z

src/client/splitor.cpp

-                            currentChunkOffset, requestLength, mdsclient,
-                            fileInfo, currentChunkIndex)) {
-            LOG(ERROR)  << "request split failed"
+    if (!isStripe) {


这里如果不是条带的情况，stripeUnit设置成chunksize，stripeCount设置为1，else里面的逻辑是不是统一的？

逻辑是统一的，我这里建议可以优化一下isStripe的判断加一个或上stripeCount=1的情况

同上。非stripe是stripe的一种特殊形式，统一处理看起来更清晰些。

为了保持原来非条带代码的独立性，将条带逻辑进行区分，但是对于stripeCount=1的场景按非条带流程走

ilixiaocui · 2021-01-20T08:44:26Z

curvefs_python/cbd_client.cpp

@@ -54,6 +54,12 @@ int CBDClient::Create(const char* filename, UserInfo_t* userInfo, size_t size) {
    return client_->Create(filename, ToCurveClientUserInfo(userInfo), size);
 }

+int CBDClient::Create2(const char* filename, UserInfo_t* userInfo, size_t size,


commit message的形式：模块名：***
client/mds：stripe feature

ilixiaocui · 2021-01-20T09:35:01Z

include/client/libcurve.h

+ * @param: filename文件名
+ * @param: userinfo是当前打开或创建时携带的user信息
+ * @param: size文件长度，当create为true的时候以size长度创建文件
+ * @param:


新增的部分都使用英文注释

ilixiaocui · 2021-01-21T09:28:11Z

src/client/libcurve_file.h

@@ -111,6 +111,21 @@ class FileClient {
                       const UserInfo_t& userinfo,
                       size_t size);

+    /**
+     * 创建带条带的文件


新增代码都使用英文注释

ilixiaocui · 2021-01-21T09:34:38Z

src/client/mds_client_base.cpp

@@ -54,6 +54,8 @@ void MDSClientBase::OpenFile(const std::string& filename,
 void MDSClientBase::CreateFile(const std::string& filename,
                               const UserInfo_t& userinfo,
                               size_t size,
+                               uint32_t stripeUnit,


这里的参数顺序跟下面不一样，调用的时候会不会容易出错
LIBCURVE_ERROR CreateFile(const std::string& filename,
const UserInfo_t& userinfo, const UserInfo_t& userinfo,
size_t size = 0, size_t size = 0,
bool normalFile = true); bool normalFile = true,
uint32_t stripeUnit = 0,
uint32_t stripeCount = 0);

ilixiaocui · 2021-01-21T09:43:39Z

src/client/splitor.cpp

-                            currentChunkOffset, requestLength, mdsclient,
-                            fileInfo, currentChunkIndex)) {
-            LOG(ERROR)  << "request split failed"
+    if (!isStripe) {


同上。非stripe是stripe的一种特殊形式，统一处理看起来更清晰些。

ilixiaocui · 2021-01-22T09:32:20Z

src/client/splitor.cpp

+    } else {
+        if (stripeCount == 1) {
+            LOG(INFO) << "stripe count is one, stripe size == chunk size";
+            stripeUnit = chunksize;


如果传入的stripeUnit跟chunksize不一致，才需要重新赋值？

可以不用判断，这一块改成设置isStripe为false

ilixiaocui · 2021-01-24T14:24:37Z

src/client/splitor.cpp

+            uint64_t curChunkOffset = blockInChunkStart + blockOff;
+            uint64_t requestLength = std::min((stripeUnit - blockOff), left);
+
+            /*LOG(INFO) << "request split curChunkIndex = " << curChunkIndex


直接删掉

ilixiaocui · 2021-01-24T14:28:39Z

src/mds/nameserver2/curvefs.cpp

+    if ((defaultChunkSize_ % stripeUnit != 0) ||
+                 (defaultChunkSize_ % stripeCount != 0)) {
+        LOG(ERROR) << "is not divisible by chunksize. stripeUnit:"
+           << stripeUnit << ",stripeCount:" << stripeCount;


这里有很多error的情况，是否只列出ok，其余返回error?

ok的条件也比较多，场景一：stripeUnit和stripeCount同时等于0。场景二：strpeUnit小于defaultChunkSize_且stripeUnit和stripeCount都能被defaultChunkSize_整除，这里还要防止输入参数其中一个为0的情况出错。我认为写起来条理没有当前清晰

ilixiaocui · 2021-02-03T08:29:15Z

include/client/libcurve.h

@@ -134,6 +134,8 @@ typedef struct FileStatInfo {
    char            filename[NAME_MAX_SIZE];
    char            owner[NAME_MAX_SIZE];
    int             fileStatus;
+    uint32_t        stripeUnit;


为什么这里stripUnit和stripeCount定义的uint32_t, 下面是uint64_t类型，下面也有很多地方是混用的

统一为uint64_t

ilixiaocui · 2021-02-03T08:31:53Z

src/client/libcurve_file.h

+     * @param: size file size
+     * @param: stripeUnit block in stripe size
+     * @param  stripeCount stripe count in one stripe
+     * @return: success return 0,  fail teturn less than 0


ilixiaocui · 2021-02-03T08:34:10Z

src/client/splitor.cpp

-                            currentChunkOffset, requestLength, mdsclient,
-                            fileInfo, currentChunkIndex)) {
-            LOG(ERROR)  << "request split failed"
+    if (!isStripe) {


isStripe的两种情况是否可以封装为两个函数：StripeHandle NormalHandle?

目前没有多处使用该部分代码逻辑，目前先不封装

ilixiaocui · 2021-02-04T03:18:59Z

src/client/splitor.cpp

+                if (!AssignInternal(iotracker, metaCache, targetlist, data,
+                                currentChunkOffset, requestLength, mdsclient,
+                                fileInfo, currentChunkIndex)) {
+                LOG(ERROR)  << "request split failed"


cm: q2: Fluentd: add project idea

baijiaruo force-pushed the stripe branch from 04e81f5 to e2319a1 Compare January 5, 2021 12:47

baijiaruo force-pushed the stripe branch from e2319a1 to 6d4b71f Compare January 13, 2021 06:44

bai-charisu reviewed Jan 15, 2021

View reviewed changes

wu-hanqing reviewed Jan 18, 2021

View reviewed changes

ilixiaocui reviewed Jan 22, 2021

View reviewed changes

ilixiaocui reviewed Jan 24, 2021

View reviewed changes

baijiaruo force-pushed the stripe branch from 6d4b71f to be81e27 Compare January 26, 2021 10:44

bai-charisu approved these changes Jan 27, 2021

View reviewed changes

wu-hanqing approved these changes Jan 28, 2021

View reviewed changes

ilixiaocui changed the title ~~merge stripe feature and unit test~~ client/mds: merge stripe feature and unit test Feb 3, 2021

ilixiaocui reviewed Feb 3, 2021

View reviewed changes

baijiaruo force-pushed the stripe branch 3 times, most recently from 0bc6e68 to 22cb6e7 Compare February 4, 2021 01:52

ilixiaocui reviewed Feb 4, 2021

View reviewed changes

baijiaruo force-pushed the stripe branch from 22cb6e7 to 1d4b2ed Compare February 4, 2021 05:38

client/mds: merge stripe feature and unit test

eb8a47d

baijiaruo force-pushed the stripe branch from 1d4b2ed to eb8a47d Compare February 5, 2021 02:26

ilixiaocui approved these changes Feb 5, 2021

View reviewed changes

ilixiaocui merged commit 70ff82d into opencurve:master Feb 5, 2021

ilixiaocui pushed a commit to ilixiaocui/curve that referenced this pull request Feb 6, 2023

Merge pull request opencurve#211 from edsiper/fluentd-q2

70a4305

cm: q2: Fluentd: add project idea

client/mds: merge stripe feature and unit test #211

client/mds: merge stripe feature and unit test #211

Conversation

baijiaruo commented Jan 5, 2021

What problem does this PR solve?

What is changed and how it works?

Check List

Choose a reason for hiding this comment

baijiaruo Jan 18, 2021 • edited Loading

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

baijiaruo Jan 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

baijiaruo Jan 19, 2021 • edited Loading

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

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

baijiaruo Jan 18, 2021 •

edited

Loading

baijiaruo Jan 19, 2021 •

edited

Loading

baijiaruo Jan 19, 2021 •

edited

Loading