DAOS-16930 pool: Share map bulk resources #15763

liw · 2025-01-22T14:50:55Z

Improve concurrent POOL_QUERY, POOL_CONNECT, and POOL_TGT_QUERY_MAP efficiency by giving them a chance to share the same pool map buffer and pool map buffer bulk handle.

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 · 2025-01-22T14:51:20Z

Ticket title is 'Pool query fail on some pool with error "DER_NOMEM(-1009): Out of memory"'
Status is 'In Progress'
Labels: 'ALCF,post_acceptance_issues'
https://daosio.atlassian.net/browse/DAOS-16930

daosbuild1 · 2025-01-22T14:56:14Z

Test stage Build on Leap 15.5 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15763/1/execution/node/370/log

daosbuild1 · 2025-01-22T14:59:31Z

Test stage Build RPM on EL 9 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15763/1/execution/node/301/log

daosbuild1 · 2025-01-22T15:01:20Z

Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15763/1/execution/node/305/log

daosbuild1 · 2025-01-22T15:01:51Z

Test stage Build RPM on Leap 15.5 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15763/1/execution/node/261/log

daosbuild1 · 2025-01-22T15:07:14Z

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-15763/1/execution/node/345/log

NiuYawei · 2025-01-23T02:04:32Z

src/pool/srv_target.c

+{
+	D_ASSERT(dss_get_module_info()->dmi_xs_id == 0);
+
+	/* We could cache this longer, actually. */


yeah, I don't quite see why we have to invalidate the bulk cache here? I think we just need to invalid it when changing pool map, no? (the change you made oin ds_pool_tgt_map_update()).

also not quite understand why map_bc_put() 2 times below if (pool->sp_map_bc == map_bc)? then the cache is invalidated even no pool map change right?

Yes, there's problem. Let me update...

the logic looks correct although. current code looks just don't want to ping the buf if no active query in long time.

liuxuezhao · 2025-01-23T02:33:18Z

src/pool/srv_target.c

+
+	D_ASSERT(dss_get_module_info()->dmi_xs_id == 0);
+
+	/* For accessing pool->sp_map, but not really necessary. */


right, on XS0 and no yield, the lock seems not must to take. but looks fine.

liuxuezhao · 2025-01-23T02:46:52Z

src/pool/srv_target.c

+{
+	D_ASSERT(dss_get_module_info()->dmi_xs_id == 0);
+
+	/* We could cache this longer, actually. */


also not quite understand why map_bc_put() 2 times below if (pool->sp_map_bc == map_bc)? then the cache is invalidated even no pool map change right?

liuxuezhao · 2025-01-23T03:01:09Z

src/pool/srv_target.c

+{
+	D_ASSERT(dss_get_module_info()->dmi_xs_id == 0);
+
+	/* We could cache this longer, actually. */


the logic looks correct although. current code looks just don't want to ping the buf if no active query in long time.

Improve concurrent POOL_QUERY, POOL_CONNECT, and POOL_TGT_QUERY_MAP efficiency by giving them a chance to share the same pool map buffer and pool map buffer bulk handle. Signed-off-by: Li Wei <liwei@hpe.com> Required-githooks: true

daosbuild1 · 2025-01-23T06:26:59Z

Test stage NLT on EL 8.8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-15763/4/testReport/

Introduce pool space query on service leader to avoid space query flooding. The pool space cache expiration time is 2 seconds by default, one can change the expiration time via DAOS_POOL_SPACE_CACHE_INTVL, if the expiration time is set to zero, space cache will be disabled. Signed-off-by: Niu Yawei <yawei.niu@intel.com>

wangshilong · 2025-01-23T09:07:19Z

src/pool/srv_target.c

+
+static void
+map_bc_put(struct ds_pool_map_bc *map_bc)
+{


[minor] D_ASSERT(map_bc->pic_ref > 0)

gnailzenh · 2025-01-23T13:24:33Z

src/pool/srv_target.c

+	if (pool->sp_map_bc == NULL) {
+		int rc;
+
+		rc = map_bc_create(ctx, pool->sp_map, &pool->sp_map_bc);


is read lock OK in this case?

as the code only execute on XS0 and no yield, looks need not lock

…karound

daosbuild1 · 2025-01-23T19:03:08Z

Test stage Functional Hardware Medium Verbs Provider completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-15763/5/testReport/

Serialize pool space query when space cache is enabled. Change CI global config to disable pool space cache since some tests may need to verify instant pool space changes. Signed-off-by: Niu Yawei <yawei.niu@hpe.com> Co-authored-by: Xuezhao Liu <xuezhao.liu@hpe.com> Co-authored-by: Liang Zhen <liang.zhen@hpe.com>

daltonbohning

pylint needs to be resolved

Skip-build: true Signed-off-by: Dalton Bohning <dalton.bohning@hpe.com>

daltonbohning · 2025-01-24T16:33:51Z

pylint needs to be resolved

Previous CI run was good:
https://build.hpdd.intel.com/blue/organizations/jenkins/daos-stack%2Fdaos/detail/PR-15763/6/pipeline

I pushed a simple fix for the lint with most CI skipped

fixed

liw force-pushed the liw/enomem-workaround branch from 8b8e8fb to 41f89fd Compare January 23, 2025 01:34

NiuYawei reviewed Jan 23, 2025

View reviewed changes

liuxuezhao reviewed Jan 23, 2025

View reviewed changes

liuxuezhao previously approved these changes Jan 23, 2025

View reviewed changes

liw dismissed liuxuezhao’s stale review via 43e0391 January 23, 2025 05:41

liw force-pushed the liw/enomem-workaround branch from 41f89fd to 43e0391 Compare January 23, 2025 05:41

DAOS-16930 pool: Share map bulk resources

2491a37

Improve concurrent POOL_QUERY, POOL_CONNECT, and POOL_TGT_QUERY_MAP efficiency by giving them a chance to share the same pool map buffer and pool map buffer bulk handle. Signed-off-by: Li Wei <liwei@hpe.com> Required-githooks: true

liw force-pushed the liw/enomem-workaround branch from 43e0391 to 2491a37 Compare January 23, 2025 05:45

NiuYawei mentioned this pull request Jan 23, 2025

DAOS-16930 pool: pool space cache #15773

Closed

18 tasks

liuxuezhao previously approved these changes Jan 23, 2025

View reviewed changes

wangshilong reviewed Jan 23, 2025

View reviewed changes

src/pool/srv_target.c

static void

map_bc_put(struct ds_pool_map_bc *map_bc)

{

Copy link

Contributor

wangshilong Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] D_ASSERT(map_bc->pic_ref > 0)

wangshilong previously approved these changes Jan 23, 2025

View reviewed changes

wangshilong requested a review from NiuYawei January 23, 2025 09:13

gnailzenh reviewed Jan 23, 2025

View reviewed changes

Merge remote-tracking branch 'origin/release/2.6' into liw/enomem-wor…

8fbbd0f

…karound

NiuYawei dismissed stale reviews from wangshilong and liuxuezhao via c0764a7 January 24, 2025 04:18

NiuYawei marked this pull request as ready for review January 24, 2025 04:18

NiuYawei requested review from a team as code owners January 24, 2025 04:18

daltonbohning previously requested changes Jan 24, 2025

View reviewed changes

fix lint

a14b94f

Skip-build: true Signed-off-by: Dalton Bohning <dalton.bohning@hpe.com>

daltonbohning requested review from wangshilong, liuxuezhao and gnailzenh January 24, 2025 16:40

mchaarawi merged commit 0f05b2f into release/2.6 Jan 24, 2025
36 of 40 checks passed

mchaarawi deleted the liw/enomem-workaround branch January 24, 2025 17:02

jolivier23 mentioned this pull request Jan 27, 2025

Merge release/2.6 into google/2.6 #15794

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAOS-16930 pool: Share map bulk resources #15763

DAOS-16930 pool: Share map bulk resources #15763

liw commented Jan 22, 2025

github-actions bot commented Jan 22, 2025

daosbuild1 commented Jan 22, 2025

daosbuild1 commented Jan 22, 2025

daosbuild1 commented Jan 22, 2025

daosbuild1 commented Jan 22, 2025

daosbuild1 commented Jan 22, 2025

NiuYawei Jan 23, 2025

liuxuezhao Jan 23, 2025

liw Jan 23, 2025

liuxuezhao Jan 23, 2025 •

edited

Loading

liuxuezhao Jan 23, 2025

liuxuezhao Jan 23, 2025

liuxuezhao Jan 23, 2025 •

edited

Loading

daosbuild1 commented Jan 23, 2025

wangshilong Jan 23, 2025

gnailzenh Jan 23, 2025

liuxuezhao Jan 23, 2025

daosbuild1 commented Jan 23, 2025

daltonbohning left a comment

daltonbohning commented Jan 24, 2025


		D_ASSERT(dss_get_module_info()->dmi_xs_id == 0);

		/* For accessing pool->sp_map, but not really necessary. */

DAOS-16930 pool: Share map bulk resources #15763

DAOS-16930 pool: Share map bulk resources #15763

Conversation

liw commented Jan 22, 2025

Before requesting gatekeeper:

Gatekeeper:

github-actions bot commented Jan 22, 2025

daosbuild1 commented Jan 22, 2025

daosbuild1 commented Jan 22, 2025

daosbuild1 commented Jan 22, 2025

daosbuild1 commented Jan 22, 2025

daosbuild1 commented Jan 22, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liuxuezhao Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liuxuezhao Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

daosbuild1 commented Jan 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daosbuild1 commented Jan 23, 2025

daltonbohning left a comment

Choose a reason for hiding this comment

daltonbohning commented Jan 24, 2025

liuxuezhao Jan 23, 2025 •

edited

Loading

liuxuezhao Jan 23, 2025 •

edited

Loading