DAOS-15739 engine: Add multi-socket support #14234

jolivier23 · 2024-04-23T21:40:01Z

Add a simple multi-socket mode for use cases where a single engine must be used. Avoids the issue of having all helper xstreams automatically assigned to a single NUMA node thus increasing efficiency of synchronizations between I/O and helper xstreams.

It is the default behavior if all of the following are true

Neither pinned_numa_node nor first_core are used.
No oversubscription is requested
NUMA has uniform number of cores
targets and helpers divide evenly among numa nodes
There is more than one numa node

Update server config logic to ensure first_core is passed on to engine if it's set while keeping existing behavior
when both first_core: 0 and pinned_numa_node are set.

Required-githooks: true

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:

Add a simple multi-socket mode for use cases where a single engine must be used. Avoids the issue of having all helper xstreams automatically assigned to a single NUMA node thus increasing efficiency of synchronizations between I/O and helper xstreams. Usage: set DAOS_MULTISOCKET=1 in server yaml to enable this mode Limitations: 1. IO xstreams and helper xstreams must each divide evenly by the number of numa cores. 2. No DAOS_OVERSUBSCRIBE is not allowed 3. Must be equal number of cores on each numa node. If DAOS_MULTISOCKET is not set, old behavior is maintained Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

github-actions · 2024-04-23T21:54:22Z

Ticket title is 'support a single engine, multi-socket configuration'
Status is 'In Review'
https://daosio.atlassian.net/browse/DAOS-15739

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

jolivier23 · 2024-04-23T22:12:48Z

Note to reviewers, I don't think this actually changes anything with respect to existing algorithms (outside of pulling the numa information for the whole node at startup). Everything else should work as before. This just gives me something to play with.

Make multi-socket the default behavior. Keep old IOFW behavior of scheduling on another IO core Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

jolivier23 · 2024-04-24T15:27:28Z

Ok, nevermind. At Johann's behest, I changed it to be default behavior where possible.

Option to bypass the forward to another xstream Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Required-githooks: true

daosbuild1 · 2024-04-24T16:57:57Z

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-14234/6/execution/node/333/log

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

NiuYawei

LGTM, though I foresee a hard merge when updating the multiprovider branch next time, hope the multiprovider branch could be landed soon. ;)

NiuYawei · 2024-04-25T02:56:29Z

docs/admin/env_variables.md

@@ -52,6 +52,7 @@ Environment variables in this section only apply to the server side.
 |DAOS\_DTX\_AGG\_THD\_AGE|DTX aggregation age threshold in seconds. The valid range is [210, 1830]. The default value is 630.|
 |DAOS\_DTX\_RPC\_HELPER\_THD|DTX RPC helper threshold. The valid range is [18, unlimited). The default value is 513.|
 |DAOS\_DTX\_BATCHED\_ULT\_MAX|The max count of DTX batched commit ULTs. The valid range is [0, unlimited). 0 means to commit DTX synchronously. The default value is 32.|
+|DAOS\_FORWARD\_SELF|Set to disable I/O forwarding on neighbor xstream in the absence of helper threads.|


I'm wondering if we should make it as default? I don't quite see the advantage of forwarding by neighbor vos xtream (if we assume the workload is balanced over server targets).

Possibly, for now I just wanted to keep it consistent with old behavior.

I changed the default in new patch and fixed one issue I hit where first engine on same node got different behavior because it used first_core: 0.

NiuYawei · 2024-04-25T03:02:08Z

src/engine/ult.c

+	uint32_t                 target;
+
+	if (dss_tgt_offload_xs_nr == 0) {
+		if (xs_type == DSS_XS_IOFW && !dss_forward_self) {


why don't apply the same to DSS_XS_OFFLOAD?

I had similar questions. Not sure why.

Address some review comments Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

daosbuild1 · 2024-04-27T12:28:56Z

Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-14234/8/execution/node/1406/log

daosbuild1 · 2024-05-01T09:02:15Z

Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-14234/9/execution/node/1465/log

doesn't omit telling the engine it was explicitly set. Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

daosbuild1 · 2024-05-01T17:07:47Z

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

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

daosbuild1 · 2024-05-01T17:36:57Z

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

when pinned_numa_node is set Features: control Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

daltonbohning · 2024-05-01T17:49:30Z

src/tests/ftest/util/server_utils_params.py

@@ -480,7 +480,7 @@ def __init__(self, base_namespace, index, provider=None, max_storage_tiers=MAX_S
        #   log_file:               map to D_LOG_FILE env
        #   env_vars:               influences DAOS I/O Engine behavior
        self.targets = BasicParameter(None, 8)
-        self.first_core = BasicParameter(None, 0)
+        self.first_core = BasicParameter(None)


This means all tests will no longer have first_core in the config. Running pr + Features: control ObjectMetadata is probably good coverage of this, but are there any areas we should be concerned with?

I'm going to change this to allow setting first_core: 0

daltonbohning · 2024-05-01T17:49:58Z

src/tests/ftest/server/metadata.yaml

This isn't a pr or control test so you'll want to include ObjectMetadata in testing

daosbuild1 · 2024-05-01T18:13:58Z

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

This reverts commit 5e6a225.

…st_core" This reverts commit e3e3a5a.

This is the simplest path forward for now. I mimic the old behavior when both are set. Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Features: control Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

daosbuild1 · 2024-05-01T20:31:27Z

Test stage NLT on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-14234/15/display/redirect

Allow-unstable-test: true Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

tanabarr

LGTM

tanabarr · 2024-05-02T10:46:14Z

src/control/server/engine/config.go

@@ -612,7 +612,7 @@ func (c *Config) WithHelperStreamCount(count int) *Config {

 // WithServiceThreadCore sets the core index to be used for running DAOS service threads.
 func (c *Config) WithServiceThreadCore(idx int) *Config {
-	c.ServiceThreadCore = idx
+	c.ServiceThreadCore = &idx


while we are changing things it might make sense to change ServiceThreadCore to *uint

tanabarr · 2024-05-02T11:54:24Z

src/engine/ult.c

+		return DSS_XS_SELF;
+	}
+
+	socket  = tgt_id / dss_numa_nr;


nit: this assignment could be specified just once at the beginning of the function rather than in two places

Backport for the following patches DAOS-13380 engine: refine tgt_nr check DAOS-15739 engine: Add multi-socket support (#14234) * DAOS-13380 engine: refine tgt_nr check 1. for non-DAOS_TARGET_OVERSUBSCRIBE case fail to start engine if #cores is not enough 2. for DAOS_TARGET_OVERSUBSCRIBE case allow to force start engine The #nr_xs_helpers possibly be reduced for either case. * DAOS-15739 engine: Add multi-socket support (#14234) Add a simple multi-socket mode for use cases where a single engine must be used. Avoids the issue of having all helper xstreams automatically assigned to a single NUMA node thus increasing efficiency of synchronizations between I/O and helper xstreams. It is the default behavior if all of the following are true Neither pinned_numa_node nor first_core are used. No oversubscription is requested NUMA has uniform number of cores targets and helpers divide evenly among numa nodes There is more than one numa node Update server config logic to ensure first_core is passed on to engine if it's set while keeping existing behavior when both first_core: 0 and pinned_numa_node are set. Signed-off-by: Jeff Olivier <[email protected]> Signed-off-by: Xuezhao Liu <[email protected]> Signed-off-by: Tom Nabarro <[email protected]>

Backport for the following patches DAOS-13380 engine: refine tgt_nr check (#12405) DAOS-15739 engine: Add multi-socket support (#14234) DAOS-623 engine: Fix a typo (#14329) * DAOS-13380 engine: refine tgt_nr check 1. for non-DAOS_TARGET_OVERSUBSCRIBE case fail to start engine if #cores is not enough 2. for DAOS_TARGET_OVERSUBSCRIBE case allow to force start engine The #nr_xs_helpers possibly be reduced for either case. * DAOS-15739 engine: Add multi-socket support (#14234) Add a simple multi-socket mode for use cases where a single engine must be used. Avoids the issue of having all helper xstreams automatically assigned to a single NUMA node thus increasing efficiency of synchronizations between I/O and helper xstreams. It is the default behavior if all of the following are true Neither pinned_numa_node nor first_core are used. No oversubscription is requested NUMA has uniform number of cores targets and helpers divide evenly among numa nodes There is more than one numa node Update server config logic to ensure first_core is passed on to engine if it's set while keeping existing behavior when both first_core: 0 and pinned_numa_node are set. Signed-off-by: Jeff Olivier <[email protected]> Signed-off-by: Xuezhao Liu <[email protected]> Signed-off-by: Tom Nabarro <[email protected]>

jolivier23 requested review from liuxuezhao, NiuYawei and Nasf-Fan April 23, 2024 21:58

minor fix

b706c29

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Remove DAOS_MULTISOCKET envirable

610a9a8

Make multi-socket the default behavior. Keep old IOFW behavior of scheduling on another IO core Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

jolivier23 requested a review from johannlombardi April 24, 2024 15:29

jolivier23 marked this pull request as ready for review April 24, 2024 15:54

Add DAOS_FORWARD_SELF

4b1730c

Option to bypass the forward to another xstream Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

jolivier23 requested a review from a team as a code owner April 24, 2024 16:08

Merge branch 'master' into jvolivie/add_multisocket

e153977

Required-githooks: true

Skip-build-ubuntu20-rpm: true

e86d14d

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

NiuYawei previously approved these changes Apr 25, 2024

View reviewed changes

jolivier23 added 2 commits April 25, 2024 07:05

Merge branch 'master' into jvolivie/add_multisocket

34459d0

Fix a bug with dss_core_offset

89a7e24

Address some review comments Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

jolivier23 dismissed NiuYawei’s stale review via 89a7e24 April 25, 2024 13:33

jolivier23 requested a review from NiuYawei April 25, 2024 13:35

NiuYawei previously approved these changes Apr 28, 2024

View reviewed changes

Merge branch 'master' into jvolivie/add_multisocket

00fcc86

johannlombardi previously approved these changes Apr 30, 2024

View reviewed changes

Merge branch 'master' into jvolivie/add_multisocket

e854c9f

jolivier23 changed the title ~~DAOS-15739 engine: Add multi-socket support~~ DAOS-15739 engine: Add multi-socket support DO NOT LAND YET May 1, 2024

jolivier23 added 2 commits May 1, 2024 10:22

Fix first_core handling in control plane so it

62d6591

doesn't omit telling the engine it was explicitly set. Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Avoid invalid assertion

043c5dc

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

jolivier23 dismissed NiuYawei’s stale review via 043c5dc May 1, 2024 16:38

jolivier23 requested review from a team as code owners May 1, 2024 16:38

jolivier23 requested review from johannlombardi and NiuYawei May 1, 2024 16:40

jolivier23 changed the title ~~DAOS-15739 engine: Add multi-socket support DO NOT LAND YET~~ DAOS-15739 engine: Add multi-socket support May 1, 2024

autoconfig shouldn't be setting both pinned_numa_node and first_core

e3e3a5a

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Fix up some configs to avoid setting first_core

5e6a225

when pinned_numa_node is set Features: control Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

jolivier23 requested review from a team as code owners May 1, 2024 17:46

daltonbohning reviewed May 1, 2024

View reviewed changes

jolivier23 added 7 commits May 1, 2024 12:31

Revert "Fix up some configs to avoid setting first_core"

a2cdd98

This reverts commit 5e6a225.

Revert "autoconfig shouldn't be setting both pinned_numa_node and fir…

9dfe48f

…st_core" This reverts commit e3e3a5a.

Allow first_core: 0 to be set with pinned_numa_node

b61c48e

This is the simplest path forward for now. I mimic the old behavior when both are set. Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Features: control

ef9c4a5

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Print which setting is superfluous

b8420f9

Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Set first core to nil

a0a86df

Features: control Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Add one comment

e0ccb53

Features: control Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

Features: control

33c7d85

Allow-unstable-test: true Required-githooks: true Signed-off-by: Jeff Olivier <[email protected]>

tanabarr approved these changes May 2, 2024

View reviewed changes

johannlombardi approved these changes May 2, 2024

View reviewed changes

jolivier23 merged commit b1e0be0 into master May 2, 2024
52 checks passed

jolivier23 deleted the jvolivie/add_multisocket branch May 2, 2024 19:43

jolivier23 mentioned this pull request May 3, 2024

DAOS-15739 engine: Add single-engine, multi-socket support #14311

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAOS-15739 engine: Add multi-socket support #14234

DAOS-15739 engine: Add multi-socket support #14234

jolivier23 commented Apr 23, 2024 •

edited

Loading

github-actions bot commented Apr 23, 2024 •

edited

Loading

jolivier23 commented Apr 23, 2024

jolivier23 commented Apr 24, 2024

daosbuild1 commented Apr 24, 2024

NiuYawei left a comment

NiuYawei Apr 25, 2024

jolivier23 Apr 25, 2024

jolivier23 Apr 25, 2024

NiuYawei Apr 25, 2024

jolivier23 Apr 25, 2024

daosbuild1 commented Apr 27, 2024

daosbuild1 commented May 1, 2024

daosbuild1 commented May 1, 2024

daosbuild1 commented May 1, 2024

daltonbohning May 1, 2024

jolivier23 May 1, 2024

daltonbohning May 1, 2024

daosbuild1 commented May 1, 2024

daosbuild1 commented May 1, 2024

tanabarr left a comment

tanabarr May 2, 2024

tanabarr May 2, 2024

DAOS-15739 engine: Add multi-socket support #14234

DAOS-15739 engine: Add multi-socket support #14234

Conversation

jolivier23 commented Apr 23, 2024 • edited Loading

Before requesting gatekeeper:

Gatekeeper:

github-actions bot commented Apr 23, 2024 • edited Loading

jolivier23 commented Apr 23, 2024

jolivier23 commented Apr 24, 2024

daosbuild1 commented Apr 24, 2024

NiuYawei left a 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

daosbuild1 commented Apr 27, 2024

daosbuild1 commented May 1, 2024

daosbuild1 commented May 1, 2024

daosbuild1 commented May 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daosbuild1 commented May 1, 2024

daosbuild1 commented May 1, 2024

tanabarr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jolivier23 commented Apr 23, 2024 •

edited

Loading

github-actions bot commented Apr 23, 2024 •

edited

Loading