DAOS-14491: Retain support for phase-1 DAV heap #13158

sherintg · 2023-10-11T12:54:58Z

A copy of the phase-1 DAV allocator with some cleanup is moved to the subdirectory src/common/dav_v1. This allocator is built as a standalone shared library and linked to the libdaos_common_pmem library. The umem will now support one more mode DAOS_MD_BMEM_V1 which results in umem instance using phase-1 DAV allocator interface and layout.

Before requesting gatekeeper:

Two review approvals and any prior change requests have been resolved.
Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
Commit messages follows the guidelines outlined here.
Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

github-actions · 2023-10-11T12:55:23Z

Bug-tracker data:
Errors are component not formatted correctly,Ticket number suffix is not a number. See https://daosio.atlassian.net/wiki/spaces/DC/pages/11133911069/Commit+Comments,Unable to load ticket data
https://daosio.atlassian.net/browse/DAOS-14491:

daosbuild1

LGTM. No errors found by checkpatch.

daosbuild1 · 2023-10-11T15:47:23Z

Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13158/5/execution/node/393/log

utils/rpms/daos.spec

daosbuild1

LGTM. No errors found by checkpatch.

daosbuild1

LGTM. No errors found by checkpatch.

daosbuild1 · 2023-10-16T08:10:15Z

Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13158/7/execution/node/365/log

The phase-2 DAV allocator is placed under the subdirectory src/common/dav_v2. This allocator is built as a standalone shared library and linked to the libdaos_common_pmem library. The umem will now support one more mode DAOS_MD_BMEM_V2. Setting this mode in umem instance will result in using phase-2 DAV allocator interfaces. Signed-off-by: Sherin T George <sherin-t.george@hpe.com>

daosbuild1

LGTM. No errors found by checkpatch.

brianjmurrell · 2023-10-17T14:20:05Z

sherintg force-pushed the sherintg/vos_on_blob_p2/DAOS-14491 branch from 8de15c5 to 4a0bede yesterday

@sherintg We generally ask people not to force-push once reviewing has started as it voids the reviewers ability to incrementally review only the changes since their last review.

brianjmurrell · 2023-10-17T14:21:17Z

src/common/dav/bucket.c

@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
-/* Copyright 2015-2023, Intel Corporation */
+/* Copyright 2015-2022, Intel Corporation */


Suggested change

/* Copyright 2015-2022, Intel Corporation */

/* Copyright 2015-2023, Intel Corporation */

yes?

brianjmurrell · 2023-10-17T14:21:36Z

src/common/dav/bucket.h

@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
-/* Copyright 2015-2023, Intel Corporation */
+/* Copyright 2015-2021, Intel Corporation */


Suggested change

/* Copyright 2015-2021, Intel Corporation */

/* Copyright 2015-2023, Intel Corporation */

brianjmurrell · 2023-10-17T14:21:53Z

src/common/dav/dav.h

@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
-/* Copyright 2015-2023, Intel Corporation */
+/* Copyright 2015-2022, Intel Corporation */


Suggested change

/* Copyright 2015-2022, Intel Corporation */

/* Copyright 2015-2023, Intel Corporation */

brianjmurrell · 2023-10-17T14:22:59Z

src/common/dav/heap.c

@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
-/* Copyright 2015-2023, Intel Corporation */
+/* Copyright 2015-2022, Intel Corporation */


Suggested change

/* Copyright 2015-2022, Intel Corporation */

/* Copyright 2015-2023, Intel Corporation */

I won't comment on any more erroneous copyright date changes but please review all of your changes for this and correct.

brianjmurrell · 2023-10-17T14:24:27Z

utils/rpms/daos.spec

@@ -585,6 +586,10 @@ getent passwd daos_agent >/dev/null || useradd -s /sbin/nologin -r -g daos_agent
 # No files in a shim package

 %changelog
+* Wed Oct 16 2023 Sherin T George <sherin-t.george@hpe.com> 2.5.100-10


Suggested change

* Wed Oct 16 2023 Sherin T George <sherin-t.george@hpe.com> 2.5.100-10

* Mon Oct 16 2023 Sherin T George <sherin-t.george@hpe.com> 2.5.100-10

perhaps?

daosbuild1

LGTM. No errors found by checkpatch.

Addressed review comments and increased the timeout of bdev unit test with memcheck to accomodate new tests added. Signed-off-by: Sherin T George <sherin-t.george@hpe.com>

daosbuild1

LGTM. No errors found by checkpatch.

sherintg · 2023-10-18T05:54:39Z

@brianjmurrell I have addressed your review comments. Apologies for making multiple force pulls after your initial review comments. There was a major design change requested and I had to redo the entire coding from the beginning.

brianjmurrell · 2023-10-18T12:49:54Z

@brianjmurrell I have addressed your review comments.

Thanks. I will re-review. But you seem to have force-pushed again:

sherintg force-pushed the sherintg/vos_on_blob_p2/DAOS-14491 branch from a66d9a3 to 0f3d386 7 hours ago

Ultimately, you want to avoid the force-push to be kind to your reviewers and allow them the efficiency of only having to review your new commits since their last review. This gets more and more important the bigger a PR gets. If you force people to have to review entire large PRs over and over again due to the force pushes you are likely to alienate your reviewers and make them want to procrastinate your review requests as you make their job more difficult that it needs to be. So ultimately it's in your interest to make reviewing as easy and efficient as possible for your reviewers.

Apologies for making multiple force pulls after your initial review comments. There was a major design change requested and I had to redo the entire coding from the beginning.

Even a major design change is not a reason to force-push (once reviews have started). Once reviews have started, just let the commits stack up and they will be squashed when landed. People review PRs as a whole (or in groups of commits in the case of incrementally reviewing), not individual commits so the number of commits does not matter.

brianjmurrell

Packaging changes look good. I will leave +1 slots for those more familiar with the bulk of the changes in this PR.

Requested changes have been made

NiuYawei · 2023-10-19T06:40:12Z

Jenkinsfile

@@ -956,7 +956,7 @@ pipeline {
                    }
                    steps {
                        job_step_update(
-                            unitTest(timeout_time: 60,
+                            unitTest(timeout_time: 120,


What's the change for?

I am running vos_tests -A 50 and bio_ut twice now. Once with default bmem and the second time with v2 bmem. Please refer changes to utils/utest.yaml

…OS-14491

Propagated umem cache changes to dav_v2 after a merge from feature branch. Signed-off-by: Sherin T George <sherin-t.george@hpe.com>

daosbuild1

LGTM. No errors found by checkpatch.

* DAOS-13701: Memory bucket allocator API definition (#13152) - New umem macros are exported to do the allocation within memory bucket. umem internally now calls the modified backend allocator routines with memory bucket id passed as argument. - umem_get_mb_evictable() and dav_get_zone_evictable() are added to support allocator returning preferred zone to be used as evictable memory bucket for current allocations. Right now these routines always return zero. - The dav heap runtime is cleaned up to make provision for memory bucket implementation. * DAOS-13703 umem: umem cache APIs for phase II (#13138) Four sets of umem cache APIs will be exported for md-on-ssd phase II: 1. Cache initialization & finalization - umem_cache_alloc() - umem_cache_free() 2. Cache map, load and pin - umem_cache_map(); - umem_cache_load(); - umem_cache_pin(); - umem_cache_unpin(); 3. Offset and memory address converting - umem_cache_off2ptr(); - umem_cache_ptr2off(); 4. Misc - umem_cache_commit(); - umem_cache_reserve(); * DAOS-14491: Retain support for phase-1 DAV heap (#13158) The phase-2 DAV allocator is placed under the subdirectory src/common/dav_v2. This allocator is built as a standalone shared library and linked to the libdaos_common_pmem library. The umem will now support one more mode DAOS_MD_BMEM_V2. Setting this mode in umem instance will result in using phase-2 DAV allocator interfaces. * DAOS-15681 bio: store scm_sz in SMD (#14330) In md-on-ssd phase 2, the scm_sz (VOS file size) could be smaller than the meta_sz (meta blob size), then we need to store an extra scm_sz in SMD, so that on engine start, this scm_sz could be retrieved from SMD for VOS file re-creation. To make the SMD compatible with pmem & md-on-ssd phase 1, a new table named "meta_pool_ex" is introduced for storing scm_sz. * DAOS-14422 control: Update pool create UX for MD-on-SSD phase2 (#14740) Show MD-on-SSD specific output on pool create and add new syntax to specify ratio between SSD capacity reserved for MD in new DAOS pool and the (static) size of memory reserved for MD in the form of VOS index files (previously held on SCM but now in tmpfs on ramdisk). Memory-file size is now printed when creating a pool in MD-on--SSD mode. The new --{meta,data}-size params can be specified in decimal or binary units e.g. GB or GiB and refer to per-rank allocations. These manual size parameters are only for advanced use cases and in most situations the --size (X%|XTB|XTiB) syntax is recommended when creating a pool. --meta-size param is bytes to use for metadata on SSD and --data-size is for data on SSD (similar to --nvme-size). The new --mem-ratio param is specified as a percentage with up to two decimal places precision. This defines the proportion of the metadata capacity reserved on SSD (i.e. --meta-size) that will be used when allocating the VOS-index (one blob and one memory file per target). Enable MD-on-SSD phase2 pool creation requires envar DAOS_MD_ON_SSD_MODE=3 to be set in server config file. * DAOS-14317 vos: initial changes for the phase2 object pre-load (#15001) - Introduced new durable format 'vos_obj_p2_df' for the md-on-ssd phase2 object, at most 4 evict-able bucket IDs could be stored. - Changed vos_obj_hold() & vos_obj_release() to pin or unpin object respectively. - Changed the private data of VOS dkey/akey/value trees from 'vos_pool' to 'vos_object', the private data will be used for allocating/reserving from the evict-able bucket. - Move the vos_obj_hold() call from vos_update_end() to vos_update_begin() for the phase2 pool, reserve value from the object evict-able bucket. * DAOS-14316 vos: object preload for GC (#15059) - Use the reserved vos_gc_item.it_args to store 2 bucket IDs for GC_OBJ, GC_DKEY and GC_AKEY, so that GC drain will be able to tell the what buckets need be pinned by looking up bucket numbers stored in vos_obj_df. - Once GC drain needs to pin a different bucket, it will have to commit current tx; unpin current bucket; pin required bucket; start new tx; - Forge a dummy object as the private data for the btree opened by GC, so that the 'ti_destroy' hack could be removed. - Store evict-able bucket ID persistently for newly created object, this was missed in prior PR. * DAOS-14315 vos: Pin objects for DTX commit & CPD RPC (#15118) Introduced two new VOS APIs vos_pin_objects() & vos_unpin_objects() for pin or unpin objects. Changed DTX commit/abort & CPD RPC handler code to ensure objects pinned before starting local transaction. - Bug fix in vos_pmemobj_create(), the actual scm_size should be passed to bio_mc_create(). - Use vos_obj_acquire() instead of vos_obj_hold() in vos_update_begin() to avoid the complication of object ilog adding in ts_set. We could simplify it in future cleanup PRs. - Handle concurrent object bucket alloting & loading. * DAOS-16160 control: Update pool create --size % opt for MD-on-SSD p2 (#14957) Update calculation of usable pool META and DATA component sizes for MD-on-SSD phase-2 mode; when meta-blob-size > vos-file-size. - Use mem-ratio when making NVMe size adjustments to calculate usable pool capacity from raw stats. - Use mem-ratio when auto-sizing to determine META component from percentage of usable rank-RAM-disk capacity. - Apportion cluster count reductions to SSDs based on number of assigned targets to take account of target striping across a tier. - Fix pool query ftest. - Improve test coverage for meta and rdb size calculations. * DAOS-16763 common: Tunable to control max NEMB (#15422) A new tunable, DAOS_MD_ON_SSD_NEMB_PCT is introuced, to define the percentage of memory cache that non-evictable memory buckets can expand to. This tunable will be read during pool creation and persisted, ensuring that each time the pool is reopened, it retains the value set during its creation. Signed-off-by: Niu Yawei <yawei.niu@intel.com> Signed-off-by: Tom Nabarro <tom.nabarro@intel.com> Signed-off-by: Sherin T George <sherin-t.george@hpe.com> Co-authored-by: Tom Nabarro <tom.nabarro@intel.com> Co-authored-by: sherintg <sherin-t.george@hpe.com>

sherintg requested review from a team as code owners October 11, 2023 12:54

sherintg self-assigned this Oct 11, 2023

This comment was marked as outdated.

Sign in to view

sherintg force-pushed the sherintg/vos_on_blob_p2/DAOS-14491 branch from a99168b to 04279b1 Compare October 11, 2023 13:05

This comment was marked as outdated.

Sign in to view

sherintg force-pushed the sherintg/vos_on_blob_p2/DAOS-14491 branch from 04279b1 to f546187 Compare October 11, 2023 13:14

This comment was marked as outdated.

Sign in to view

sherintg force-pushed the sherintg/vos_on_blob_p2/DAOS-14491 branch from f546187 to b228965 Compare October 11, 2023 14:41

sherintg requested a review from a team as a code owner October 11, 2023 14:41

This comment was marked as outdated.

Sign in to view

sherintg force-pushed the sherintg/vos_on_blob_p2/DAOS-14491 branch from b228965 to 6635df6 Compare October 11, 2023 15:15

daosbuild1 reviewed Oct 11, 2023

View reviewed changes

daos-stack deleted a comment from daosbuild1 Oct 11, 2023

brianjmurrell reviewed Oct 11, 2023

View reviewed changes

utils/rpms/daos.spec Show resolved Hide resolved

daosbuild1 reviewed Oct 12, 2023

View reviewed changes

sherintg force-pushed the sherintg/vos_on_blob_p2/DAOS-14491 branch from 8acd938 to 8de15c5 Compare October 16, 2023 07:57

daosbuild1 reviewed Oct 16, 2023

View reviewed changes

sherintg force-pushed the sherintg/vos_on_blob_p2/DAOS-14491 branch from 8de15c5 to 4a0bede Compare October 16, 2023 08:44

daosbuild1 reviewed Oct 16, 2023

View reviewed changes

sherintg requested review from NiuYawei, jolivier23 and brianjmurrell October 16, 2023 09:37

jolivier23 approved these changes Oct 16, 2023

View reviewed changes

brianjmurrell previously requested changes Oct 17, 2023

View reviewed changes

daosbuild1 reviewed Oct 18, 2023

View reviewed changes

DAOS-14491: Retain support for phase-1 DAV heap

0f3d386

Addressed review comments and increased the timeout of bdev unit test with memcheck to accomodate new tests added. Signed-off-by: Sherin T George <sherin-t.george@hpe.com>

sherintg force-pushed the sherintg/vos_on_blob_p2/DAOS-14491 branch from a66d9a3 to 0f3d386 Compare October 18, 2023 05:30

daosbuild1 reviewed Oct 18, 2023

View reviewed changes

sherintg requested a review from brianjmurrell October 18, 2023 05:57

brianjmurrell reviewed Oct 18, 2023

View reviewed changes

NiuYawei approved these changes Oct 19, 2023

View reviewed changes

sherintg added 2 commits October 19, 2023 17:06

Merge branch 'feature/vos_on_blob_p2' into sherintg/vos_on_blob_p2/DA…

3a4df68

…OS-14491

DAOS-14491: Retain support for phase-1 DAV heap

ee2f2e8

Propagated umem cache changes to dav_v2 after a merge from feature branch. Signed-off-by: Sherin T George <sherin-t.george@hpe.com>

daosbuild1 reviewed Oct 19, 2023

View reviewed changes

sherintg requested a review from a team October 24, 2023 03:54

NiuYawei merged commit d99eae7 into feature/vos_on_blob_p2 Oct 25, 2023
15 of 16 checks passed

NiuYawei deleted the sherintg/vos_on_blob_p2/DAOS-14491 branch October 25, 2023 01:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAOS-14491: Retain support for phase-1 DAV heap #13158

DAOS-14491: Retain support for phase-1 DAV heap #13158

sherintg commented Oct 11, 2023

github-actions bot commented Oct 11, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

daosbuild1 left a comment

daosbuild1 commented Oct 11, 2023

daosbuild1 left a comment

daosbuild1 left a comment

daosbuild1 commented Oct 16, 2023

daosbuild1 left a comment

brianjmurrell commented Oct 17, 2023

brianjmurrell Oct 17, 2023

brianjmurrell Oct 17, 2023

brianjmurrell Oct 17, 2023

brianjmurrell Oct 17, 2023

brianjmurrell Oct 17, 2023

daosbuild1 left a comment

daosbuild1 left a comment

sherintg commented Oct 18, 2023

brianjmurrell commented Oct 18, 2023 •

edited

Loading

brianjmurrell left a comment

NiuYawei Oct 19, 2023

sherintg Oct 19, 2023

daosbuild1 left a comment

	/* Copyright 2015-2022, Intel Corporation */
	/* Copyright 2015-2023, Intel Corporation */

	/* Copyright 2015-2021, Intel Corporation */
	/* Copyright 2015-2023, Intel Corporation */

	* Wed Oct 16 2023 Sherin T George <sherin-t.george@hpe.com> 2.5.100-10
	* Mon Oct 16 2023 Sherin T George <sherin-t.george@hpe.com> 2.5.100-10

DAOS-14491: Retain support for phase-1 DAV heap #13158

DAOS-14491: Retain support for phase-1 DAV heap #13158

Conversation

sherintg commented Oct 11, 2023

Before requesting gatekeeper:

Gatekeeper:

github-actions bot commented Oct 11, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 commented Oct 11, 2023

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 commented Oct 16, 2023

daosbuild1 left a comment

Choose a reason for hiding this comment

brianjmurrell commented Oct 17, 2023

brianjmurrell Oct 17, 2023

Choose a reason for hiding this comment

brianjmurrell Oct 17, 2023

Choose a reason for hiding this comment

brianjmurrell Oct 17, 2023

Choose a reason for hiding this comment

brianjmurrell Oct 17, 2023

Choose a reason for hiding this comment

brianjmurrell Oct 17, 2023

Choose a reason for hiding this comment

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 left a comment

Choose a reason for hiding this comment

sherintg commented Oct 18, 2023

brianjmurrell commented Oct 18, 2023 • edited Loading

brianjmurrell left a comment

Choose a reason for hiding this comment

NiuYawei Oct 19, 2023

Choose a reason for hiding this comment

sherintg Oct 19, 2023

Choose a reason for hiding this comment

daosbuild1 left a comment

Choose a reason for hiding this comment

brianjmurrell commented Oct 18, 2023 •

edited

Loading