Adopt device type #167

xiaoxichen · 2024-04-12T10:41:03Z

No description provided.

src/lib/homestore_backend/hs_homeobject.cpp

conanfile.py

Signed-off-by: Xiaoxi Chen <[email protected]>

https://jubianchi.github.io/semver-check/#/^6.2/6.5 Caret constraint Constraint will be satisfied by versions matching >=6.2.0 <7.0.0-0. ^6.2 is a caret constraint. It means that it will match several versions. Given the constraint you entered, you will get: The next minor releases which will provide new features The next patch releases which will fix bugs ^6.2 is a range constraint. It means that it will match several versions. Signed-off-by: Xiaoxi Chen <[email protected]>

yamingk · 2024-04-18T16:29:12Z

conanfile.py

@@ -40,8 +40,8 @@ def build_requirements(self):
        self.build_requires("gtest/1.14.0")

    def requirements(self):
-        self.requires("homestore/[>=6.2,      include_prerelease=True]@oss/master")
-        self.requires("sisl/[>=12.1, include_prerelease=True]@oss/master")
+        self.requires("homestore/[^6.2, include_prerelease=True]@oss/master")


question: if HS made some api change that will break HO build, previously we only bump minor version, this stays true still, right? e.g. we don't need to bump major, just bump minor for HS.

I am not sure...previously we bumped the major version. We discussed in HS meeting but I am not sure a decision has been made.

If that is the case, we need to pin to [^6.2.0]

src/lib/homestore_backend/hs_homeobject.cpp

yamingk · 2024-04-18T16:39:36Z

src/lib/homestore_backend/hs_homeobject.cpp

+                {HS_SERVICE::REPLICATION,
+                 hs_format_params{.dev_type = HSDevType::Data,
+                                  .size_pct = 99.0,
+                                  .num_chunks = 65000,


Shouldn't num_chunks from NVME drives + num_chunks from HDD equal to 65000?

Is it because maximum is 64K which is 65536, and we leave the 536 for NVME chunks?

yes that is the idea, as said, lower it to 60000 to give more chunks to nvme

yamingk · 2024-04-18T16:54:12Z

src/lib/homestore_backend/hs_homeobject.cpp

+            HomeStore::instance()->format_and_start({
+                {HS_SERVICE::META, hs_format_params{.dev_type = HSDevType::Fast, .size_pct = 9.0, .num_chunks = 64}},
+                {HS_SERVICE::LOG,
+                 hs_format_params{.dev_type = HSDevType::Fast, .size_pct = 45.0, .chunk_size = 32 * Mi}},


I understand if chunk_size is specified, HS starts from 0 num_chunks, but how does it know how many chunks has been created in total, and what is the available number chunks this LOG service can use, is there a place we assert that the total number chunks created is less than 64K (this is that FIXME you've put in creaet_vdev, right?).

I did some calculation, say one NVME drive is 500GB, 45% is around 200GB and it will require 6400 number of chunks with 32MB chunk size. 6400 + 64 + 128 + 65000 will exceed 64K, right? Please correct me if I missed something here.

yes this is very tricky part. The log device go with chunk_size and it creates chunk dynamically, so worth case it could create FAST_SIZE * PCT / CHUNK_SIZE . But we dont know have the intelligent to distribute #chunks across service, that is part of the fixme.

For the configuration now I tend to believe the behavior will be only 344 chunks is availalbe for log, bounding max logstore size to 344*32MB = 11G.

I will reduce the 65000 to 60000 for now. Other enhancement can be taken care later.

So the ideal solution (with your change, in homestore) seems to be, log device should use that chunk_size, and num_chunks should not exceeding the total 64K, regardless of the pct set for this log service. This will result in losing some space not being used, better than creating chunk numbers that will overflow the chunk number and cause correctness issues.

yes I think we need to change the log vdev , when creating a max_num_chunks parameterscan be provided and logstore need to stay within the limit to avoid create_chunk failure, also ensure flushing/truncating the logdev properly based on the max_num_chunks. Logstore can take this chance to decide whether it want to adjust the chunk_size.

There will not be correctness issue as in pdev::create_chunk , it will throw std::out_of_range but that probably crash the process. https://github.com/eBay/HomeStore/blob/2c500c573fa2c5733218e0cbdd44226fb3e6504f/src/lib/device/physical_dev.cpp#L275

yamingk · 2024-04-19T01:19:02Z

src/lib/homestore_backend/hs_homeobject.cpp

+                {HS_SERVICE::REPLICATION,
+                 hs_format_params{.dev_type = run_on_type,
+                                  .size_pct = 79.0,
+                                  .num_chunks = 65000,


this should also be adjusted, right? 10 pct of total device would be still very large and exceed 64K in total?

Also can we be more conservative say setting this to 40000, before the fixme part is done? I am not sure what will be the total disk size in production, we probably need some careful calculation.

For mixed mode, I remember hearing something from John D saying we would have around 900GB of nvme per SM, and it would result in around 12800 num_chunks for logstore with 32MB chunk_size.

yes, lets postpone this calculation and will get it from testing on real environment.

that give 5536 chunks for NVME. Signed-off-by: Xiaoxi Chen <[email protected]>

Signed-off-by: Xiaoxi Chen <[email protected]>

cxxopts use >> to parse opts. Signed-off-by: Xiaoxi Chen <[email protected]>

yamingk · 2024-04-19T16:34:29Z

conanfile.py

@@ -40,8 +40,8 @@ def build_requirements(self):
        self.build_requires("gtest/1.14.0")

    def requirements(self):
-        self.requires("homestore/[>=6.2,      include_prerelease=True]@oss/master")
-        self.requires("sisl/[>=12.1, include_prerelease=True]@oss/master")
+        self.requires("homestore/[~6.2.0, include_prerelease=True]@oss/master")


This should be ~6.2, right? So that HO can still pick any patch changes automatcially from HS?
Or is ~6.2.0 also do the work?

Signed-off-by: Xiaoxi Chen <[email protected]>

xiaoxichen commented Apr 12, 2024

View reviewed changes

src/lib/homestore_backend/hs_homeobject.cpp Outdated Show resolved Hide resolved

xiaoxichen requested review from hkadayam, yamingk and JacksonYao287 April 12, 2024 18:10

JacksonYao287 reviewed Apr 13, 2024

View reviewed changes

src/lib/homestore_backend/hs_homeobject.cpp Show resolved Hide resolved

JacksonYao287 reviewed Apr 13, 2024

View reviewed changes

conanfile.py Outdated Show resolved Hide resolved

JacksonYao287 mentioned this pull request Apr 15, 2024

fix io_uring error eBay/HomeStore#380

Closed

xiaoxichen force-pushed the dev_type branch 4 times, most recently from 1e133ac to 187a74f Compare April 18, 2024 03:30

xiaoxichen added 7 commits April 18, 2024 09:07

Adopt device type

6e6fa72

Signed-off-by: Xiaoxi Chen <[email protected]>

UT with hybrid mode

fc8e161

Signed-off-by: Xiaoxi Chen <[email protected]>

fix a bug

c68746d

Signed-off-by: Xiaoxi Chen <[email protected]>

fix ut

f7c8620

Signed-off-by: Xiaoxi Chen <[email protected]>

Sanity check for no supported device

251ae9e

Signed-off-by: Xiaoxi Chen <[email protected]>

add operator ==

476fa06

Signed-off-by: Xiaoxi Chen <[email protected]>

xiaoxichen force-pushed the dev_type branch from d4b691b to 00ab48e Compare April 18, 2024 16:08

yamingk reviewed Apr 18, 2024

View reviewed changes

src/lib/homestore_backend/hs_homeobject.cpp Show resolved Hide resolved

yamingk reviewed Apr 18, 2024

View reviewed changes

yamingk reviewed Apr 19, 2024

View reviewed changes

reduce #chunk on data to be 60000

ff62e8f

that give 5536 chunks for NVME. Signed-off-by: Xiaoxi Chen <[email protected]>

xiaoxichen force-pushed the dev_type branch from cf553b9 to ff62e8f Compare April 19, 2024 02:20

limit HS to below 6.3

8436d39

Signed-off-by: Xiaoxi Chen <[email protected]>

xiaoxichen force-pushed the dev_type branch from 6910d0f to 4c40fac Compare April 19, 2024 06:23

add formattr and >> operator for SM command line parsing.

aa43265

cxxopts use >> to parse opts. Signed-off-by: Xiaoxi Chen <[email protected]>

xiaoxichen force-pushed the dev_type branch from 4c40fac to aa43265 Compare April 19, 2024 11:16

yamingk previously approved these changes Apr 19, 2024

View reviewed changes

yamingk reviewed Apr 19, 2024

View reviewed changes

update conan with tilde constraint

be41347

Signed-off-by: Xiaoxi Chen <[email protected]>

xiaoxichen dismissed yamingk’s stale review via be41347 April 19, 2024 16:43

yamingk approved these changes Apr 19, 2024

View reviewed changes

xiaoxichen merged commit 7dda682 into eBay:main Apr 19, 2024
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adopt device type #167

Adopt device type #167

xiaoxichen commented Apr 12, 2024

yamingk Apr 18, 2024

xiaoxichen Apr 19, 2024

yamingk Apr 18, 2024 •

edited

Loading

xiaoxichen Apr 19, 2024 •

edited

Loading

yamingk Apr 18, 2024

xiaoxichen Apr 19, 2024 •

edited

Loading

yamingk Apr 19, 2024 •

edited

Loading

xiaoxichen Apr 19, 2024

yamingk Apr 19, 2024 •

edited

Loading

xiaoxichen Apr 19, 2024

yamingk Apr 19, 2024

Adopt device type #167

Adopt device type #167

Conversation

xiaoxichen commented Apr 12, 2024

yamingk Apr 18, 2024

Choose a reason for hiding this comment

xiaoxichen Apr 19, 2024

Choose a reason for hiding this comment

yamingk Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

xiaoxichen Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

yamingk Apr 18, 2024

Choose a reason for hiding this comment

xiaoxichen Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

yamingk Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

xiaoxichen Apr 19, 2024

Choose a reason for hiding this comment

yamingk Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

xiaoxichen Apr 19, 2024

Choose a reason for hiding this comment

yamingk Apr 19, 2024

Choose a reason for hiding this comment

yamingk Apr 18, 2024 •

edited

Loading

xiaoxichen Apr 19, 2024 •

edited

Loading

xiaoxichen Apr 19, 2024 •

edited

Loading

yamingk Apr 19, 2024 •

edited

Loading

yamingk Apr 19, 2024 •

edited

Loading