diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md new file mode 100644 index 00000000000..2f777d8f41d --- /dev/null +++ b/PR_DESCRIPTION.md @@ -0,0 +1,193 @@ +# Add allow-numa-imbalance flag to config generate + +## Summary + +This PR adds a new `--allow-numa-imbalance` / `-n` flag to the `dmg config generate` and `daos_server config generate` commands. This flag enables generation of server configurations where NVMe devices are distributed equally across engines regardless of NUMA affinity. + +## Problem Statement + +Currently, the config generation commands enforce balanced NVMe device distribution that respects NUMA affinity. When generating a config: + +1. The tool calculates the lowest common number of SSDs across all NUMA nodes +2. It restricts each engine to use only that minimum number of SSDs from its local NUMA node +3. Extra SSDs on NUMA nodes with more devices are excluded from the config + +This prevents maximizing device utilization in heterogeneous hardware environments where: +- Different NUMA nodes have different numbers of NVMe devices +- Hardware constraints result in uneven device distribution +- Users want to utilize all available SSDs even if it requires cross-NUMA access + +## Solution + +Add a `--allow-numa-imbalance` flag that: + +1. **Collects all SSDs**: Gathers all available NVMe devices from all NUMA nodes +2. **Distributes equally**: Allocates SSDs evenly across all engines, ignoring NUMA boundaries +3. **Handles remainders**: Uses maximum divisible number of SSDs; remainder SSDs are not used +4. **Warns about performance**: Flag description includes warning about suboptimal performance +5. **Sets server config parameter**: The generated config includes `allow_numa_imbalance: true` to allow the server to start with this configuration +6. **Maintains backward compatibility**: The default behavior (without the flag) remains unchanged + +### Example Behavior + +**Before (without flag) - NUMA-aware balancing:** +- NUMA-0: 4 SSDs available → 2 SSDs used (limited to minimum) +- NUMA-1: 2 SSDs available → 2 SSDs used +- Total: 4 of 6 SSDs used + +**After (with flag) - Equal distribution:** +- NUMA-0: 4 SSDs available → 3 SSDs assigned (may include devices from NUMA-1) +- NUMA-1: 2 SSDs available → 3 SSDs assigned (may include devices from NUMA-0) +- Total: All 6 SSDs used, distributed equally + +**With remainder (7 SSDs / 2 engines):** +- All 7 SSDs collected, but only 6 used (3 per engine) +- 1 SSD remains unused (not evenly divisible) +- Warning logged: "using 6 SSDs (3 per engine), 1 SSDs will not be used" + +## Changes Made + +### 1. Command-line Interface (`src/control/common/cmdutil/auto.go`) +- Added `AllowNumaImbalance bool` field to `ConfGenCmd` struct +- Added short flag `-n` and long flag `--allow-numa-imbalance` +- Flag description: "Redistribute NVMe devices equally across engines regardless of NUMA affinity" + +### 2. Control Library (`src/control/lib/control/auto.go`) +- Added `AllowNumaImbalance bool` field to `ConfGenerateReq` struct +- Modified `correctSSDCounts()` function to accept `allowImbalance` parameter + - When `false`, performs NUMA-aware balancing (existing behavior) + - When `true`, calls new `distributeSSDs()` function +- Added new `distributeSSDs()` function: + - Collects all SSDs from all NUMA nodes + - Validates that VMD devices are not present (cannot distribute VMD) + - Calculates SSDs per engine (total / nrEngines) + - If remainder exists, uses only maximum divisible number + - Logs notice when SSDs are unused due to remainder + - Distributes SSDs equally across engines + - Logs detailed information about distribution +- Updated `genEngineConfigs()` to pass the flag to `correctSSDCounts()` +- Modified `genServerConfig()` to call `WithAllowNumaImbalance()` on generated config + +### 3. Tests (`src/control/lib/control/auto_test.go`) +- Updated `TestControl_AutoConfig_correctSSDCounts` to test the new parameter +- Added test case "allow imbalance distributes equally": + - 4 SSDs on NUMA-0 + 2 on NUMA-1 = 6 total + - Distributes to 3 SSDs per engine + - Verifies equal distribution +- Added test case "allow imbalance with remainder discards extras": + - 5 SSDs on NUMA-0 + 2 on NUMA-1 = 7 total + - Uses 6 SSDs (3 per engine), 1 SSD unused + - Verifies remainder handling +- Added test case "allow imbalance with 8 SSDs across 2 engines": + - 5 SSDs on NUMA-0 + 3 on NUMA-1 = 8 total + - Distributes to 4 SSDs per engine + - Verifies equal distribution works +- Existing test cases verify default behavior still works correctly + +### 4. Documentation (`docs/admin/deployment.md`) +- Added flag to both `daos_server config generate` and `dmg config generate` help output +- Added detailed description explaining: + - Default NUMA-aware balancing behavior + - Effect of the flag (equal distribution regardless of NUMA) + - Handling of remainders (maximum divisible number used) + - **Performance warning**: May result in suboptimal performance + - That it sets `allow_numa_imbalance: true` in the generated config + +## Use Cases + +This feature is useful for: + +1. **Development/testing environments** with heterogeneous hardware +2. **Hardware upgrades** where NVMe devices are added incrementally to specific nodes +3. **Hardware failures** where some devices are unavailable on certain NUMA nodes +4. **Specialized configurations** requiring maximum utilization despite imbalance +5. **Cost-optimized deployments** with mixed hardware specifications + +## Example Usage + +### Before (without flag) +```bash +$ daos_server config generate +# NUMA-0: 4 SSDs available, 2 used (limited to minimum) +# NUMA-1: 2 SSDs available, 2 used +# Total: 4 of 6 SSDs utilized +``` + +### After (with flag) +```bash +$ daos_server config generate --allow-numa-imbalance +# Collects all 6 SSDs and redistributes equally: +# - Engine 0: Gets 3 SSDs (regardless of which NUMA node they're on) +# - Engine 1: Gets 3 SSDs (regardless of which NUMA node they're on) +# - Config includes: allow_numa_imbalance: true +# Total: All 6 SSDs utilized +``` + +### DMG remote generation +```bash +$ dmg config generate -l host1,host2 --allow-numa-imbalance +# Generates config allowing imbalanced NVMe distribution across the hostset +``` + +## Testing + +### Unit Tests +- Added test case for `correctSSDCounts()` with imbalance flag +- Verified existing tests still pass (default behavior unchanged) + +### Manual Testing +Run the following to verify: +```bash +# Test without flag (default behavior) +daos_server config generate + +# Test with flag (new behavior) +daos_server config generate --allow-numa-imbalance + +# Verify generated config contains: allow_numa_imbalance: true +``` + +## Backward Compatibility + +✅ **Fully backward compatible** +- Default behavior unchanged (flag defaults to `false`) +- Existing scripts and automation continue to work +- No breaking changes to API or config format + +## Future Enhancements + +Potential follow-up work: +1. Add validation to warn users about performance implications of imbalanced configs +2. Provide recommendations for device distribution optimization +3. Add metrics to monitor imbalance effects on production systems + +## Checklist + +- [x] Code changes implemented +- [x] Unit tests added/updated +- [x] Documentation updated +- [x] Commit message follows guidelines +- [x] Signed-off-by line included +- [ ] CI/CD tests pass +- [ ] Review by control plane experts + +## Related Issues + +- Addresses user requests for config generation in heterogeneous environments +- Related to DAOS-16979 (hugepage/NUMA imbalance handling) + +--- + +**Testing Instructions for Reviewers:** + +1. Verify flag appears in help output: + ```bash + daos_server config generate --help | grep numa + dmg config generate --help | grep numa + ``` + +2. Test with imbalanced hardware (if available) or mock setup + +3. Verify generated config includes `allow_numa_imbalance: true` when flag is used + +4. Confirm default behavior (without flag) still balances SSDs correctly diff --git a/docs/admin/deployment.md b/docs/admin/deployment.md index 633a48a83c8..29381f4db12 100644 --- a/docs/admin/deployment.md +++ b/docs/admin/deployment.md @@ -298,6 +298,8 @@ Help Options: MD-on-SSD config -f, --fabric-ports= Allow custom fabric interface ports to be specified for each engine config section. Comma separated port numbers, one per engine + -n, --allow-numa-imbalance Distribute NVMe devices equally across engines regardless of NUMA + affinity (may result in suboptimal performance due to cross-NUMA access) --skip-prep Skip preparation of devices during scan. ``` @@ -344,6 +346,8 @@ Help Options: MD-on-SSD config -f, --fabric-ports= Allow custom fabric interface ports to be specified for each engine config section. Comma separated port numbers, one per engine + -n, --allow-numa-imbalance Distribute NVMe devices equally across engines regardless of NUMA + affinity (may result in suboptimal performance due to cross-NUMA access) ``` The `daos_server` service must be running on the remote storage servers and as such a minimal @@ -404,6 +408,17 @@ this option is set then a MD-on-SSD config will be generated. - `--fabric-ports` enables custom port numbers to be assigned to each engine's fabric settings. Comma separated list must contain enough numbers to cover all engines generated in config. +- `--allow-numa-imbalance` allows the generation of a server config with NVMe devices distributed +equally across engines regardless of NUMA affinity. By default, config generation respects NUMA +affinity and balances the number of SSDs assigned to each engine by limiting all engines to use +only the lowest common number of SSDs across NUMA nodes. When this flag is set, all available SSDs +are collected and distributed equally across all engines, ignoring NUMA node boundaries. If the total +number of SSDs is not evenly divisible by the number of engines, only the maximum divisible number +of SSDs are used (e.g., 7 SSDs / 2 engines = 3 per engine, 1 SSD unused). This maximizes device +utilization in heterogeneous environments but **may result in suboptimal performance** due to +cross-NUMA memory access. The generated config will include `allow_numa_imbalance: true` to allow +the server to start with this configuration. + The text generated by the command and output to stdout can be copied and used as the server config file on relevant hosts (normally by copying to `/etc/daos/daos_server.yml` and (re)starting service). diff --git a/src/control/common/cmdutil/auto.go b/src/control/common/cmdutil/auto.go index 26bd56eba49..60c2195ea52 100644 --- a/src/control/common/cmdutil/auto.go +++ b/src/control/common/cmdutil/auto.go @@ -8,14 +8,15 @@ type deprecatedParams struct { type ConfGenCmd struct { deprecatedParams - MgmtSvcReplicas string `default:"localhost" short:"r" long:"ms-replicas" description:"Comma separated list of MS replica addresses to host management service"` - NrEngines int `short:"e" long:"num-engines" description:"Set the number of DAOS Engine sections to be populated in the config file output. If unset then the value will be set to the number of NUMA nodes on storage hosts in the DAOS system."` - SCMOnly bool `short:"s" long:"scm-only" description:"Create a SCM-only config without NVMe SSDs."` - NetClass string `default:"infiniband" short:"c" long:"net-class" description:"Set the network device class to be used" choice:"ethernet" choice:"infiniband"` - NetProvider string `short:"p" long:"net-provider" description:"Set the network fabric provider to be used"` - UseTmpfsSCM bool `short:"t" long:"use-tmpfs-scm" description:"Use tmpfs for scm rather than PMem"` - ExtMetadataPath string `short:"m" long:"control-metadata-path" description:"External storage path to store control metadata. Set this to a persistent location and specify --use-tmpfs-scm to create an MD-on-SSD config"` - FabricPorts string `short:"f" long:"fabric-ports" description:"Allow custom fabric interface ports to be specified for each engine config section. Comma separated port numbers, one per engine"` + MgmtSvcReplicas string `default:"localhost" short:"r" long:"ms-replicas" description:"Comma separated list of MS replica addresses to host management service"` + NrEngines int `short:"e" long:"num-engines" description:"Set the number of DAOS Engine sections to be populated in the config file output. If unset then the value will be set to the number of NUMA nodes on storage hosts in the DAOS system."` + SCMOnly bool `short:"s" long:"scm-only" description:"Create a SCM-only config without NVMe SSDs."` + NetClass string `default:"infiniband" short:"c" long:"net-class" description:"Set the network device class to be used" choice:"ethernet" choice:"infiniband"` + NetProvider string `short:"p" long:"net-provider" description:"Set the network fabric provider to be used"` + UseTmpfsSCM bool `short:"t" long:"use-tmpfs-scm" description:"Use tmpfs for scm rather than PMem"` + ExtMetadataPath string `short:"m" long:"control-metadata-path" description:"External storage path to store control metadata. Set this to a persistent location and specify --use-tmpfs-scm to create an MD-on-SSD config"` + FabricPorts string `short:"f" long:"fabric-ports" description:"Allow custom fabric interface ports to be specified for each engine config section. Comma separated port numbers, one per engine"` + AllowNumaImbalance bool `short:"n" long:"allow-numa-imbalance" description:"Distribute NVMe devices equally across engines regardless of NUMA affinity (may result in suboptimal performance due to cross-NUMA access)"` } // CheckDeprecated will check for deprecated parameters and update as needed. diff --git a/src/control/lib/control/auto.go b/src/control/lib/control/auto.go index d07a0a3e9c7..ae79ebc4ce9 100644 --- a/src/control/lib/control/auto.go +++ b/src/control/lib/control/auto.go @@ -70,8 +70,10 @@ type ( // Generate config with a tmpfs RAM-disk SCM. UseTmpfsSCM bool `json:"UseTmpfsSCM"` // Location to persist control-plane metadata, will generate MD-on-SSD config. - ExtMetadataPath string `json:"ExtMetadataPath"` - Log logging.Logger `json:"-"` + ExtMetadataPath string `json:"ExtMetadataPath"` + // Allow NUMA imbalance when NVMe devices are not evenly distributed. + AllowNumaImbalance bool `json:"AllowNumaImbalance"` + Log logging.Logger `json:"-"` } // ConfGenerateResp contains the generated server config. @@ -916,7 +918,12 @@ func filterDevicesByAffinity(req ConfGenerateReq, nd *networkDetails, sd *storag return nodeSet, nil } -func correctSSDCounts(log logging.Logger, sd *storageDetails) error { +func correctSSDCounts(log logging.Logger, sd *storageDetails, allowImbalance bool) error { + if allowImbalance { + log.Debug("allow-numa-imbalance enabled, distributing SSDs equally across engines") + return distributeSSDs(log, sd) + } + // calculate ssd count lowest value across numa nodes minSSDsInCfg, err := lowestCommonNrSSDs(sd.NumaSSDs.keys(), sd.NumaSSDs) if err != nil { @@ -950,6 +957,57 @@ func correctSSDCounts(log logging.Logger, sd *storageDetails) error { return nil } +// distributeSSDs allocates all available SSDs equally across engines, ignoring NUMA affinity. +// Each engine receives the same number of SSDs. If the total number of SSDs is not evenly +// divisible by the number of engines, only the maximum divisible number of SSDs are used and +// remainder SSDs are not included in the generated configuration. +func distributeSSDs(log logging.Logger, sd *storageDetails) error { + numaIDs := sd.NumaSSDs.keys() + if len(numaIDs) == 0 { + return errors.New("no numa nodes with SSDs found") + } + + // Collect all SSDs from all NUMA nodes + var allSSDs []string + for _, numaID := range numaIDs { + ssds := sd.NumaSSDs[numaID] + if ssds.HasVMD() { + // VMD devices cannot be distributed across NUMA nodes + log.Debugf("NUMA-%d has VMD devices, skipping distribution", numaID) + return errors.New("cannot distribute VMD devices across NUMA nodes, " + + "use without --allow-numa-imbalance flag") + } + allSSDs = append(allSSDs, ssds.Strings()...) + } + + totalSSDs := len(allSSDs) + nrEngines := len(numaIDs) + ssdsPerEngine := totalSSDs / nrEngines + remainder := totalSSDs % nrEngines + ssdsToUse := ssdsPerEngine * nrEngines + + if remainder > 0 { + log.Noticef("total SSDs (%d) not evenly divisible by engines (%d); "+ + "using %d SSDs (%d per engine), %d SSDs will not be used", + totalSSDs, nrEngines, ssdsToUse, ssdsPerEngine, remainder) + } + + log.Debugf("distributing %d SSDs equally across %d engines (%d per engine)", + ssdsToUse, nrEngines, ssdsPerEngine) + + // Distribute SSDs equally across engines, using only the divisible portion + idx := 0 + for i, numaID := range numaIDs { + engineSSDs := allSSDs[idx : idx+ssdsPerEngine] + sd.NumaSSDs[numaID] = hardware.MustNewPCIAddressSet(engineSSDs...) + log.Debugf("assigned %d SSDs to NUMA-%d (engine %d): %v", + ssdsPerEngine, numaID, i, engineSSDs) + idx += ssdsPerEngine + } + + return nil +} + func getSCMTier(log logging.Logger, numaID, nrNumaNodes int, sd *storageDetails) (*storage.TierConfig, error) { scmTier := storage.NewTierConfig().WithStorageClass(sd.scmCls.String()). WithScmMountPoint(fmt.Sprintf("%s%d", scmMountPrefix, numaID)) @@ -1056,7 +1114,7 @@ func genEngineConfigs(req ConfGenerateReq, newEngineCfg newEngineCfgFn, nodeSet // if nvme is enabled, make ssd counts consistent across numa groupings if !req.SCMOnly { - if err := correctSSDCounts(req.Log, sd); err != nil { + if err := correctSSDCounts(req.Log, sd, req.AllowNumaImbalance); err != nil { return nil, err } } @@ -1231,7 +1289,8 @@ func genServerConfig(req ConfGenerateReq, ecs []*engine.Config, tc *threadCounts WithMgmtSvcReplicas(req.MgmtSvcReplicas...). WithFabricProvider(ecs[0].Fabric.Provider). WithEngines(ecs...). - WithControlLogFile(defaultControlLogFile) + WithControlLogFile(defaultControlLogFile). + WithAllowNumaImbalance(req.AllowNumaImbalance) for idx := range cfg.Engines { tiers := cfg.Engines[idx].Storage.Tiers diff --git a/src/control/lib/control/auto_test.go b/src/control/lib/control/auto_test.go index 596b0d3c5de..54d33f45c63 100644 --- a/src/control/lib/control/auto_test.go +++ b/src/control/lib/control/auto_test.go @@ -1073,9 +1073,10 @@ func TestControl_AutoConfig_filterDevicesByAffinity(t *testing.T) { func TestControl_AutoConfig_correctSSDCounts(t *testing.T) { for name, tc := range map[string]struct { - sd storageDetails - expErr error - expSD storageDetails // expected details after updates + sd storageDetails + allowImbalance bool + expErr error + expSD storageDetails // expected details after updates }{ "no ssds": { sd: storageDetails{ @@ -1108,12 +1109,84 @@ func TestControl_AutoConfig_correctSSDCounts(t *testing.T) { }, }, }, + "allow imbalance distributes equally": { + sd: storageDetails{ + NumaSCMs: numaSCMsMap{ + 0: []string{"/dev/pmem0"}, + 1: []string{"/dev/pmem1"}, + }, + NumaSSDs: numaSSDsMap{ + 0: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(0, 1, 2, 3)...), + 1: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(4, 5)...), + }, + }, + allowImbalance: true, + expSD: storageDetails{ + NumaSCMs: numaSCMsMap{ + 0: []string{"/dev/pmem0"}, + 1: []string{"/dev/pmem1"}, + }, + // 6 total SSDs distributed equally: 3 per engine + NumaSSDs: numaSSDsMap{ + 0: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(0, 1, 2)...), + 1: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(3, 4, 5)...), + }, + }, + }, + "allow imbalance with remainder discards extras": { + sd: storageDetails{ + NumaSCMs: numaSCMsMap{ + 0: []string{"/dev/pmem0"}, + 1: []string{"/dev/pmem1"}, + }, + NumaSSDs: numaSSDsMap{ + 0: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(0, 1, 2, 3, 4)...), + 1: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(5, 6)...), + }, + }, + allowImbalance: true, + expSD: storageDetails{ + NumaSCMs: numaSCMsMap{ + 0: []string{"/dev/pmem0"}, + 1: []string{"/dev/pmem1"}, + }, + // 7 total SSDs: uses 6 (3 per engine), 1 remainder not used + NumaSSDs: numaSSDsMap{ + 0: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(0, 1, 2)...), + 1: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(3, 4, 5)...), + }, + }, + }, + "allow imbalance with 8 SSDs across 2 engines": { + sd: storageDetails{ + NumaSCMs: numaSCMsMap{ + 0: []string{"/dev/pmem0"}, + 1: []string{"/dev/pmem1"}, + }, + NumaSSDs: numaSSDsMap{ + 0: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(0, 1, 2, 3, 4)...), + 1: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(5, 6, 7)...), + }, + }, + allowImbalance: true, + expSD: storageDetails{ + NumaSCMs: numaSCMsMap{ + 0: []string{"/dev/pmem0"}, + 1: []string{"/dev/pmem1"}, + }, + // 8 total SSDs distributed equally: 4 per engine + NumaSSDs: numaSSDsMap{ + 0: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(0, 1, 2, 3)...), + 1: hardware.MustNewPCIAddressSet(test.MockPCIAddrs(4, 5, 6, 7)...), + }, + }, + }, } { t.Run(name, func(t *testing.T) { log, buf := logging.NewTestLogger(t.Name()) defer test.ShowBufferOnFailure(t, buf) - gotErr := correctSSDCounts(log, &tc.sd) + gotErr := correctSSDCounts(log, &tc.sd, tc.allowImbalance) test.CmpErr(t, tc.expErr, gotErr) if tc.expErr != nil { return