diff --git a/src/control/cmd/dmg/pool.go b/src/control/cmd/dmg/pool.go index 46fc0154042..17ec28e5e83 100644 --- a/src/control/cmd/dmg/pool.go +++ b/src/control/cmd/dmg/pool.go @@ -763,22 +763,23 @@ type poolQueryCmd struct { poolCmd HealthOnly bool `short:"t" long:"health-only" description:"Only perform pool health related queries"` ShowEnabledRanks bool `short:"e" long:"show-enabled" description:"Show engine unique identifiers (ranks) which are enabled"` - NoSelfHealCheck bool `long:"no-self-heal-check" description:"Disable self_heal pool property check"` } // Execute is run when PoolQueryCmd subcommand is activated func (cmd *poolQueryCmd) Execute(args []string) error { + selfHeal, err := daos.PoolProperties().GetProperty("self_heal") + if err != nil { + return err + } req := &control.PoolQueryReq{ - ID: cmd.PoolID().String(), - QueryMask: daos.DefaultPoolQueryMask, + ID: cmd.PoolID().String(), + QueryMask: daos.DefaultPoolQueryMask, + Properties: []*daos.PoolProperty{selfHeal}, } if cmd.HealthOnly { req.QueryMask = daos.HealthOnlyPoolQueryMask } - if !cmd.NoSelfHealCheck { - req.QueryMask.SetOptions(daos.PoolQueryOptionSelfHealPolicy) - } if cmd.ShowEnabledRanks { req.QueryMask.SetOptions(daos.PoolQueryOptionEnabledEngines) } @@ -790,7 +791,6 @@ func (cmd *poolQueryCmd) Execute(args []string) error { if resp != nil { poolInfo = &resp.PoolInfo } - // NOTE: Self heal policy info not returned in JSON response. return cmd.OutputJSON(poolInfo, err) } diff --git a/src/control/cmd/dmg/pool_test.go b/src/control/cmd/dmg/pool_test.go index 02c23ef0fc2..a259219dd33 100644 --- a/src/control/cmd/dmg/pool_test.go +++ b/src/control/cmd/dmg/pool_test.go @@ -1242,14 +1242,8 @@ func TestDmg_PoolCommands(t *testing.T) { printRequest(t, &control.PoolQueryReq{ ID: "12345678-1234-1234-1234-1234567890ab", QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) { - qm.SetOptions(daos.PoolQueryOptionSelfHealPolicy) }), - }), - printRequest(t, &control.PoolGetPropReq{ - ID: test.MockUUID(0), - Properties: []*daos.PoolProperty{ - propWithVal("self_heal", ""), - }, + Properties: []*daos.PoolProperty{propWithVal("self_heal", "")}, }), }, " "), nil, @@ -1262,14 +1256,8 @@ func TestDmg_PoolCommands(t *testing.T) { ID: "12345678-1234-1234-1234-1234567890ab", QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) { qm.SetOptions(daos.PoolQueryOptionEnabledEngines) - qm.SetOptions(daos.PoolQueryOptionSelfHealPolicy) }), - }), - printRequest(t, &control.PoolGetPropReq{ - ID: test.MockUUID(0), - Properties: []*daos.PoolProperty{ - propWithVal("self_heal", ""), - }, + Properties: []*daos.PoolProperty{propWithVal("self_heal", "")}, }), }, " "), nil, @@ -1282,14 +1270,8 @@ func TestDmg_PoolCommands(t *testing.T) { ID: "12345678-1234-1234-1234-1234567890ab", QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) { qm.SetOptions(daos.PoolQueryOptionEnabledEngines) - qm.SetOptions(daos.PoolQueryOptionSelfHealPolicy) }), - }), - printRequest(t, &control.PoolGetPropReq{ - ID: test.MockUUID(0), - Properties: []*daos.PoolProperty{ - propWithVal("self_heal", ""), - }, + Properties: []*daos.PoolProperty{propWithVal("self_heal", "")}, }), }, " "), nil, @@ -1297,32 +1279,13 @@ func TestDmg_PoolCommands(t *testing.T) { { "Query pool for health only", "pool query --health-only 12345678-1234-1234-1234-1234567890ab", - strings.Join([]string{ - printRequest(t, &control.PoolQueryReq{ - ID: "12345678-1234-1234-1234-1234567890ab", - QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) { - *qm = daos.HealthOnlyPoolQueryMask - qm.SetOptions(daos.PoolQueryOptionSelfHealPolicy) - }), - }), - printRequest(t, &control.PoolGetPropReq{ - ID: test.MockUUID(0), - Properties: []*daos.PoolProperty{ - propWithVal("self_heal", ""), - }, - }), - }, " "), - nil, - }, - { - "Query pool for health only; skip self_heal fetch", - "pool query --health-only 12345678-1234-1234-1234-1234567890ab --no-self-heal-check", strings.Join([]string{ printRequest(t, &control.PoolQueryReq{ ID: "12345678-1234-1234-1234-1234567890ab", QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) { *qm = daos.HealthOnlyPoolQueryMask }), + Properties: []*daos.PoolProperty{propWithVal("self_heal", "")}, }), }, " "), nil, @@ -1334,14 +1297,8 @@ func TestDmg_PoolCommands(t *testing.T) { printRequest(t, &control.PoolQueryReq{ ID: "test_label", QueryMask: setQueryMask(func(qm *daos.PoolQueryMask) { - qm.SetOptions(daos.PoolQueryOptionSelfHealPolicy) }), - }), - printRequest(t, &control.PoolGetPropReq{ - ID: test.MockUUID(0), - Properties: []*daos.PoolProperty{ - propWithVal("self_heal", ""), - }, + Properties: []*daos.PoolProperty{propWithVal("self_heal", "")}, }), }, " "), nil, diff --git a/src/control/cmd/dmg/pretty/pool.go b/src/control/cmd/dmg/pretty/pool.go index 1a830ce2870..14b5c16b30f 100644 --- a/src/control/cmd/dmg/pretty/pool.go +++ b/src/control/cmd/dmg/pretty/pool.go @@ -1,6 +1,6 @@ // // (C) Copyright 2020-2024 Intel Corporation. -// (C) Copyright 2025 Hewlett Packard Enterprise Development LP +// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -30,7 +30,9 @@ func PrintPoolQueryResponse(pqr *control.PoolQueryResp, out io.Writer, opts ...P if err := pretty.PrintPoolInfo(&pqr.PoolInfo, out); err != nil { return err } - pretty.PrintPoolSelfHealDisable(pqr.PoolInfo.SelfHealPolicy, pqr.SysSelfHealPolicy, out) + if pqr.SelfHealPolicy != "" { + pretty.PrintPoolSelfHealDisable(pqr.SelfHealPolicy, pqr.SysSelfHealPolicy, out) + } return nil } diff --git a/src/control/common/proto/error.go b/src/control/common/proto/error.go index 48331e482e8..e52a4ed9818 100644 --- a/src/control/common/proto/error.go +++ b/src/control/common/proto/error.go @@ -1,5 +1,6 @@ // // (C) Copyright 2020-2022 Intel Corporation. +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -34,6 +35,9 @@ const ( // AnnotatedSystemErrNotReplica defines an identifier for ErrNotReplica errors // serialized as gRPC status metadata. AnnotatedSystemErrNotReplica = "proto.system.ErrNotReplica" + // AnnotatedSystemErrNoEngine defines an identifier for ErrNoEngine + // errors serialized as gRPC status metadata. + AnnotatedSystemErrNoEngine = "proto.system.ErrNoEngine" // AnnotatedSystemErrPoolNotFound defines an identifier for ErrPoolNotFound errors // serialized as gRPC status metadata. AnnotatedSystemErrPoolNotFound = "proto.system.ErrPoolNotFound" @@ -51,6 +55,8 @@ func ErrFromMeta(meta map[string]string, errType error) error { err = json.Unmarshal(jm, et) case *system.ErrNotReplica: err = json.Unmarshal([]byte(meta["Replicas"]), &et.Replicas) + case *system.ErrNoEngine: + err = json.Unmarshal([]byte(meta["Replicas"]), &et.Replicas) case *system.ErrNotLeader: et.LeaderHint = meta["LeaderHint"] err = json.Unmarshal([]byte(meta["Replicas"]), &et.Replicas) @@ -114,6 +120,18 @@ func AnnotateError(in error) error { "Replicas": string(data), }, } + case *system.ErrNoEngine: + data, err := json.Marshal(et.Replicas) + if err != nil { + break + } + details = &errdetails.ErrorInfo{ + Reason: AnnotatedSystemErrNoEngine, + Domain: "DAOS", + Metadata: map[string]string{ + "Replicas": string(data), + }, + } case *system.ErrNotLeader: data, err := json.Marshal(et.Replicas) if err != nil { @@ -173,6 +191,8 @@ func UnwrapError(st *status.Status) error { return daos.Status(i) case AnnotatedSystemErrNotReplica: return ErrFromMeta(t.Metadata, new(system.ErrNotReplica)) + case AnnotatedSystemErrNoEngine: + return ErrFromMeta(t.Metadata, new(system.ErrNoEngine)) case AnnotatedSystemErrNotLeader: return ErrFromMeta(t.Metadata, new(system.ErrNotLeader)) case AnnotatedSystemErrPoolNotFound: diff --git a/src/control/common/proto/error_test.go b/src/control/common/proto/error_test.go index 85c5d03a805..c47f4da908b 100644 --- a/src/control/common/proto/error_test.go +++ b/src/control/common/proto/error_test.go @@ -1,5 +1,6 @@ // // (C) Copyright 2020-2022 Intel Corporation. +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -78,6 +79,9 @@ func TestProto_AnnotateError(t *testing.T) { LeaderHint: "foo.bar.baz", Replicas: []string{"a", "b", "c"}, } + testEngineUnavailable := &system.ErrNoEngine{ + Replicas: []string{"b", "c"}, + } testPoolNotFound := system.ErrPoolLabelNotFound("foo") for name, tc := range map[string]struct { @@ -111,6 +115,10 @@ func TestProto_AnnotateError(t *testing.T) { err: testNotLeader, expErr: testNotLeader, }, + "wrap/unwrap ErrNoEngine": { + err: testEngineUnavailable, + expErr: testEngineUnavailable, + }, "wrap/unwrap ErrPoolNotFound": { err: testPoolNotFound, expErr: testPoolNotFound, diff --git a/src/control/common/proto/mgmt/pool.pb.go b/src/control/common/proto/mgmt/pool.pb.go index 34f2ee1b789..4d2954b98fd 100644 --- a/src/control/common/proto/mgmt/pool.pb.go +++ b/src/control/common/proto/mgmt/pool.pb.go @@ -1592,6 +1592,7 @@ type PoolQueryReq struct { Id string `protobuf:"bytes,2,opt,name=id,proto3" json:"id,omitempty"` SvcRanks []uint32 `protobuf:"varint,3,rep,packed,name=svc_ranks,json=svcRanks,proto3" json:"svc_ranks,omitempty"` // List of pool service ranks QueryMask uint64 `protobuf:"varint,4,opt,name=query_mask,json=queryMask,proto3" json:"query_mask,omitempty"` // Bitmask of pool query options + Properties []*PoolProperty `protobuf:"bytes,5,rep,name=properties,proto3" json:"properties,omitempty"` // Pool properties to return with the query unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -1654,6 +1655,13 @@ func (x *PoolQueryReq) GetQueryMask() uint64 { return 0 } +func (x *PoolQueryReq) GetProperties() []*PoolProperty { + if x != nil { + return x.Properties + } + return nil +} + // StorageUsageStats represents usage statistics for a storage subsystem. type StorageUsageStats struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -1850,7 +1858,7 @@ type PoolQueryResp struct { MemFileBytes uint64 `protobuf:"varint,21,opt,name=mem_file_bytes,json=memFileBytes,proto3" json:"mem_file_bytes,omitempty"` // per-pool accumulated value of memory file sizes DeadRanks string `protobuf:"bytes,22,opt,name=dead_ranks,json=deadRanks,proto3" json:"dead_ranks,omitempty"` // optional set of dead ranks MdOnSsdActive bool `protobuf:"varint,23,opt,name=md_on_ssd_active,json=mdOnSsdActive,proto3" json:"md_on_ssd_active,omitempty"` // MD-on-SSD mode flag - SelfHealPolicy string `protobuf:"bytes,24,opt,name=self_heal_policy,json=selfHealPolicy,proto3" json:"self_heal_policy,omitempty"` // pool property self_heal value + Properties []*PoolProperty `protobuf:"bytes,24,rep,name=properties,proto3" json:"properties,omitempty"` // requested pool properties SysSelfHealPolicy string `protobuf:"bytes,25,opt,name=sys_self_heal_policy,json=sysSelfHealPolicy,proto3" json:"sys_self_heal_policy,omitempty"` // system property self_heal value unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache @@ -2040,11 +2048,11 @@ func (x *PoolQueryResp) GetMdOnSsdActive() bool { return false } -func (x *PoolQueryResp) GetSelfHealPolicy() string { +func (x *PoolQueryResp) GetProperties() []*PoolProperty { if x != nil { - return x.SelfHealPolicy + return x.Properties } - return "" + return nil } func (x *PoolQueryResp) GetSysSelfHealPolicy() string { @@ -3834,13 +3842,16 @@ const file_mgmt_pool_proto_rawDesc = "" + "containers\x18\x02 \x03(\v2\x17.mgmt.ListContResp.ContR\n" + "containers\x1a\x1a\n" + "\x04Cont\x12\x12\n" + - "\x04uuid\x18\x01 \x01(\tR\x04uuid\"l\n" + + "\x04uuid\x18\x01 \x01(\tR\x04uuid\"\xa0\x01\n" + "\fPoolQueryReq\x12\x10\n" + "\x03sys\x18\x01 \x01(\tR\x03sys\x12\x0e\n" + "\x02id\x18\x02 \x01(\tR\x02id\x12\x1b\n" + "\tsvc_ranks\x18\x03 \x03(\rR\bsvcRanks\x12\x1d\n" + "\n" + - "query_mask\x18\x04 \x01(\x04R\tqueryMask\"\xac\x01\n" + + "query_mask\x18\x04 \x01(\x04R\tqueryMask\x122\n" + + "\n" + + "properties\x18\x05 \x03(\v2\x12.mgmt.PoolPropertyR\n" + + "properties\"\xac\x01\n" + "\x11StorageUsageStats\x12\x14\n" + "\x05total\x18\x01 \x01(\x04R\x05total\x12\x12\n" + "\x04free\x18\x02 \x01(\x04R\x04free\x12\x10\n" + @@ -3864,7 +3875,7 @@ const file_mgmt_pool_proto_rawDesc = "" + "\aSTOPPED\x10\x04\x12\v\n" + "\aFAILING\x10\x05\x12\n" + "\n" + - "\x06FAILED\x10\x06\"\x89\a\n" + + "\x06FAILED\x10\x06\"\x93\a\n" + "\rPoolQueryResp\x12\x16\n" + "\x06status\x18\x01 \x01(\x05R\x06status\x12\x12\n" + "\x04uuid\x18\x02 \x01(\tR\x04uuid\x12\x14\n" + @@ -3891,8 +3902,10 @@ const file_mgmt_pool_proto_rawDesc = "" + "\x0emem_file_bytes\x18\x15 \x01(\x04R\fmemFileBytes\x12\x1d\n" + "\n" + "dead_ranks\x18\x16 \x01(\tR\tdeadRanks\x12'\n" + - "\x10md_on_ssd_active\x18\x17 \x01(\bR\rmdOnSsdActive\x12(\n" + - "\x10self_heal_policy\x18\x18 \x01(\tR\x0eselfHealPolicy\x12/\n" + + "\x10md_on_ssd_active\x18\x17 \x01(\bR\rmdOnSsdActive\x122\n" + + "\n" + + "properties\x18\x18 \x03(\v2\x12.mgmt.PoolPropertyR\n" + + "properties\x12/\n" + "\x14sys_self_heal_policy\x18\x19 \x01(\tR\x11sysSelfHealPolicyJ\x04\b\t\x10\n" + "R\vtotal_nodes\"\x7f\n" + "\fPoolProperty\x12\x16\n" + @@ -4108,25 +4121,27 @@ var file_mgmt_pool_proto_depIdxs = []int32{ 27, // 0: mgmt.PoolCreateReq.properties:type_name -> mgmt.PoolProperty 50, // 1: mgmt.ListPoolsResp.pools:type_name -> mgmt.ListPoolsResp.Pool 51, // 2: mgmt.ListContResp.containers:type_name -> mgmt.ListContResp.Cont - 0, // 3: mgmt.StorageUsageStats.media_type:type_name -> mgmt.StorageMediaType - 2, // 4: mgmt.PoolRebuildStatus.state:type_name -> mgmt.PoolRebuildStatus.State - 2, // 5: mgmt.PoolRebuildStatus.derived_state:type_name -> mgmt.PoolRebuildStatus.State - 25, // 6: mgmt.PoolQueryResp.rebuild:type_name -> mgmt.PoolRebuildStatus - 24, // 7: mgmt.PoolQueryResp.tier_stats:type_name -> mgmt.StorageUsageStats - 1, // 8: mgmt.PoolQueryResp.state:type_name -> mgmt.PoolServiceState - 27, // 9: mgmt.PoolSetPropReq.properties:type_name -> mgmt.PoolProperty - 27, // 10: mgmt.PoolGetPropReq.properties:type_name -> mgmt.PoolProperty - 27, // 11: mgmt.PoolGetPropResp.properties:type_name -> mgmt.PoolProperty - 0, // 12: mgmt.StorageTargetUsage.media_type:type_name -> mgmt.StorageMediaType - 3, // 13: mgmt.PoolQueryTargetInfo.state:type_name -> mgmt.PoolQueryTargetInfo.TargetState - 34, // 14: mgmt.PoolQueryTargetInfo.space:type_name -> mgmt.StorageTargetUsage - 35, // 15: mgmt.PoolQueryTargetResp.infos:type_name -> mgmt.PoolQueryTargetInfo - 4, // 16: mgmt.PoolRevokeClientReq.evict_mode:type_name -> mgmt.PoolRevokeClientReq.EvictMode - 17, // [17:17] is the sub-list for method output_type - 17, // [17:17] is the sub-list for method input_type - 17, // [17:17] is the sub-list for extension type_name - 17, // [17:17] is the sub-list for extension extendee - 0, // [0:17] is the sub-list for field type_name + 27, // 3: mgmt.PoolQueryReq.properties:type_name -> mgmt.PoolProperty + 0, // 4: mgmt.StorageUsageStats.media_type:type_name -> mgmt.StorageMediaType + 2, // 5: mgmt.PoolRebuildStatus.state:type_name -> mgmt.PoolRebuildStatus.State + 2, // 6: mgmt.PoolRebuildStatus.derived_state:type_name -> mgmt.PoolRebuildStatus.State + 25, // 7: mgmt.PoolQueryResp.rebuild:type_name -> mgmt.PoolRebuildStatus + 24, // 8: mgmt.PoolQueryResp.tier_stats:type_name -> mgmt.StorageUsageStats + 1, // 9: mgmt.PoolQueryResp.state:type_name -> mgmt.PoolServiceState + 27, // 10: mgmt.PoolQueryResp.properties:type_name -> mgmt.PoolProperty + 27, // 11: mgmt.PoolSetPropReq.properties:type_name -> mgmt.PoolProperty + 27, // 12: mgmt.PoolGetPropReq.properties:type_name -> mgmt.PoolProperty + 27, // 13: mgmt.PoolGetPropResp.properties:type_name -> mgmt.PoolProperty + 0, // 14: mgmt.StorageTargetUsage.media_type:type_name -> mgmt.StorageMediaType + 3, // 15: mgmt.PoolQueryTargetInfo.state:type_name -> mgmt.PoolQueryTargetInfo.TargetState + 34, // 16: mgmt.PoolQueryTargetInfo.space:type_name -> mgmt.StorageTargetUsage + 35, // 17: mgmt.PoolQueryTargetResp.infos:type_name -> mgmt.PoolQueryTargetInfo + 4, // 18: mgmt.PoolRevokeClientReq.evict_mode:type_name -> mgmt.PoolRevokeClientReq.EvictMode + 19, // [19:19] is the sub-list for method output_type + 19, // [19:19] is the sub-list for method input_type + 19, // [19:19] is the sub-list for extension type_name + 19, // [19:19] is the sub-list for extension extendee + 0, // [0:19] is the sub-list for field type_name } func init() { file_mgmt_pool_proto_init() } diff --git a/src/control/lib/control/pool.go b/src/control/lib/control/pool.go index 4f0d128d8e4..7c9772d6805 100644 --- a/src/control/lib/control/pool.go +++ b/src/control/lib/control/pool.go @@ -467,8 +467,9 @@ type ( // PoolQueryReq contains the parameters for a pool query request. PoolQueryReq struct { poolRequest - ID string - QueryMask daos.PoolQueryMask + ID string + QueryMask daos.PoolQueryMask + Properties []*daos.PoolProperty } // PoolQueryResp contains the pool query response. @@ -530,41 +531,6 @@ func (pqr *PoolQueryResp) UpdateState() error { return nil } -// UpdateSelfHealPolicy retrieves the value of the pool's self_heal policy and adds it to the -// response. Default policy value is returned if the property value cannot be fetched. -func (pqr *PoolQueryResp) UpdateSelfHealPolicy(ctx context.Context, rpcClient UnaryInvoker) error { - req := &PoolGetPropReq{ - ID: pqr.UUID.String(), - Properties: []*daos.PoolProperty{ - daos.PoolProperties()["self_heal"].GetProperty("self_heal"), - }, - } - - props, err := PoolGetProp(ctx, rpcClient, req) - if err != nil { - return errors.Wrapf(err, "PoolGetProp") - } - - switch len(props) { - case 0: - rpcClient.Debug("self_heal pool property not found, assuming default 'exclude;rebuild'") - pqr.SelfHealPolicy = daos.DefaultPoolSelfHealStr - case 1: - pqr.SelfHealPolicy = props[0].StringValue() - if pqr.SelfHealPolicy == "not set" { - pqr.SelfHealPolicy = daos.DefaultPoolSelfHealStr - } - default: - return errors.Errorf("unexpected number of pool props returned, want 1 got %d", - len(props)) - } - - rpcClient.Debugf("pool-query: fetched pool self_heal propval: %s (from props %+v)", - pqr.SelfHealPolicy, props) - - return nil -} - // poolQueryInt is the internal implementation of PoolQuery that // allows the caller to supply an optional pointer to the response // that will be filled out with the query details. @@ -574,6 +540,11 @@ func poolQueryInt(ctx context.Context, rpcClient UnaryInvoker, req *PoolQueryReq Id: req.ID, QueryMask: uint64(req.QueryMask), } + var err error + pbReq.Properties, err = convertPoolProps(req.Properties, false) + if err != nil { + return nil, err + } req.setRPC(func(ctx context.Context, conn *grpc.ClientConn) (proto.Message, error) { return mgmtpb.NewMgmtSvcClient(conn).PoolQuery(ctx, pbReq) }) @@ -584,10 +555,19 @@ func poolQueryInt(ctx context.Context, rpcClient UnaryInvoker, req *PoolQueryReq return nil, err } + msResp, err := ur.getMSResponse() + if err != nil { + return nil, err + } + pbResp, ok := msResp.(*mgmtpb.PoolQueryResp) + if !ok { + return nil, errors.New("unable to extract PoolQueryResp from MS response") + } + if resp == nil { resp = new(PoolQueryResp) } - if err := convertMSResponse(ur, resp); err != nil { + if err := convert.Types(pbResp, resp); err != nil { return nil, err } @@ -599,13 +579,21 @@ func poolQueryInt(ctx context.Context, rpcClient UnaryInvoker, req *PoolQueryReq return nil, err } - if req.QueryMask.HasOption(daos.PoolQueryOptionSelfHealPolicy) { - if err := resp.UpdateSelfHealPolicy(ctx, rpcClient); err != nil { - return nil, errors.Wrap(err, "pool get-prop self_heal failed") + props, err := convertPoolPropsResp(rpcClient, req.Properties, pbResp.GetProperties()) + if err != nil { + return nil, err + } + var kept []*daos.PoolProperty + for _, prop := range props { + if prop.Name == "self_heal" { + resp.SelfHealPolicy = prop.StringValue() + continue } + kept = append(kept, prop) } + resp.Properties = kept - return resp, err + return resp, nil } // PoolQuery performs a pool query operation for the specified pool ID on a @@ -784,6 +772,46 @@ type PoolGetPropReq struct { Properties []*daos.PoolProperty } +// convertPoolPropsResp matches the properties returned by the replica to the +// ones requested; values are set on the request copies. +func convertPoolPropsResp(log debugLogger, reqProps []*daos.PoolProperty, pbProps []*mgmtpb.PoolProperty) ([]*daos.PoolProperty, error) { + if len(reqProps) == 0 { + return nil, nil + } + + pbMap := make(map[uint32]*mgmtpb.PoolProperty) + for _, prop := range pbProps { + if _, found := pbMap[prop.GetNumber()]; found { + return nil, errors.Errorf("got > 1 occurrences of prop %d in resp", + prop.GetNumber()) + } + pbMap[prop.GetNumber()] = prop + } + + resp := make([]*daos.PoolProperty, 0, len(reqProps)) + for _, prop := range reqProps { + pbProp, found := pbMap[prop.Number] + if !found { + // Properties can be missing due to DAOS-11418 and DAOS-13919 + log.Debugf("can't find prop %d (%s) in resp", prop.Number, prop.Name) + continue + } + switch v := pbProp.GetValue().(type) { + case *mgmtpb.PoolProperty_Strval: + prop.Value.SetString(v.Strval) + case *mgmtpb.PoolProperty_Numval: + prop.Value.SetNumber(v.Numval) + case *mgmtpb.PoolProperty_Byteval: + prop.Value.SetBytes(v.Byteval) + default: + return nil, errors.Errorf("unable to represent response value %+v", v) + } + resp = append(resp, prop) + } + + return resp, nil +} + // PoolGetProp sends a pool get-prop request to the pool service leader. func PoolGetProp(ctx context.Context, rpcClient UnaryInvoker, req *PoolGetPropReq) ([]*daos.PoolProperty, error) { if len(req.Properties) == 0 { @@ -834,38 +862,7 @@ func PoolGetProp(ctx context.Context, rpcClient UnaryInvoker, req *PoolGetPropRe return nil, errors.New("unable to extract PoolGetPropResp from MS response") } - pbMap := make(map[uint32]*mgmtpb.PoolProperty) - for _, prop := range pbResp.GetProperties() { - if _, found := pbMap[prop.GetNumber()]; found { - return nil, errors.Errorf("got > 1 occurrences of prop %d in resp", - prop.GetNumber()) - } - pbMap[prop.GetNumber()] = prop - } - - // We've already sorted req.Properties above. - resp := make([]*daos.PoolProperty, 0, len(req.Properties)) - for _, prop := range req.Properties { - pbProp, found := pbMap[prop.Number] - if !found { - // Properties can be missing due to DAOS-11418 and DAOS-13919 - rpcClient.Debugf("can't find prop %d (%s) in resp", prop.Number, prop.Name) - continue - } - switch v := pbProp.GetValue().(type) { - case *mgmtpb.PoolProperty_Strval: - prop.Value.SetString(v.Strval) - case *mgmtpb.PoolProperty_Numval: - prop.Value.SetNumber(v.Numval) - case *mgmtpb.PoolProperty_Byteval: - prop.Value.SetBytes(v.Byteval) - default: - return nil, errors.Errorf("unable to represent response value %+v", v) - } - resp = append(resp, prop) - } - - return resp, nil + return convertPoolPropsResp(rpcClient, req.Properties, pbResp.GetProperties()) } // PoolRanksReq struct contains request for operation on multiple pool-ranks. Control API receives diff --git a/src/control/lib/control/pool_test.go b/src/control/lib/control/pool_test.go index 152fb64cc7d..f5e6c78fb82 100644 --- a/src/control/lib/control/pool_test.go +++ b/src/control/lib/control/pool_test.go @@ -1564,92 +1564,6 @@ func TestControl_PoolQueryResp_UnmarshalJSON(t *testing.T) { } } -func TestControl_PoolQueryResp_UpdateSelfHealPolicy(t *testing.T) { - type prop struct { - number uint32 - value interface{} - } - makePropResp := func(props ...prop) *mgmtpb.PoolGetPropResp { - pbProps := make([]*mgmtpb.PoolProperty, 0, len(props)) - for _, p := range props { - switch v := p.value.(type) { - case string: - pbProps = append(pbProps, &mgmtpb.PoolProperty{ - Number: p.number, - Value: &mgmtpb.PoolProperty_Strval{Strval: v}, - }) - case int: - pbProps = append(pbProps, &mgmtpb.PoolProperty{ - Number: p.number, - Value: &mgmtpb.PoolProperty_Numval{Numval: uint64(v)}, - }) - } - } - return &mgmtpb.PoolGetPropResp{ - Properties: pbProps, - } - } - selfHealPropNum := propWithVal("self_heal", "").Number - - for name, tc := range map[string]struct { - getPropResp *mgmtpb.PoolGetPropResp - getPropErr error - expValue string - expErr string - }{ - "no properties returned": { - getPropResp: makePropResp(), // no properties - expValue: "exclude;rebuild", - }, - "single string value; not set value ignored": { - getPropResp: makePropResp(prop{selfHealPropNum, "rebuild"}), - expValue: "exclude;rebuild", - }, - "single num value": { - getPropResp: makePropResp(prop{selfHealPropNum, daos.PoolSelfHealingAutoRebuild}), - expValue: "rebuild", - }, - "multiple properties returned": { - getPropResp: makePropResp( - prop{selfHealPropNum, daos.PoolSelfHealingAutoRebuild}, - prop{selfHealPropNum, daos.PoolSelfHealingAutoExclude}, - ), - expErr: "> 1 occurrences of prop 4", - }, - "get-prop returns error": { - getPropErr: errors.New("something bad"), - expErr: "something bad", - }, - } { - t.Run(name, func(t *testing.T) { - log, buf := logging.NewTestLogger(t.Name()) - defer test.ShowBufferOnFailure(t, buf) - - mic := &MockInvokerConfig{ - UnaryResponseSet: []*UnaryResponse{ - MockMSResponse("host1", tc.getPropErr, tc.getPropResp), - }, - } - resp := &PoolQueryResp{} - gotErr := resp.UpdateSelfHealPolicy(context.Background(), - NewMockInvoker(log, mic)) - - var expErr error - if tc.expErr != "" { - expErr = errors.New(tc.expErr) - } - test.CmpErr(t, expErr, gotErr) - if expErr != nil { - return - } - - if resp.SelfHealPolicy != tc.expValue { - t.Errorf("expected SelfHealPolicy %q, got %q", tc.expValue, resp.SelfHealPolicy) - } - }) - } -} - func TestControl_PoolQuery(t *testing.T) { poolUUID := test.MockPoolUUID() @@ -1965,18 +1879,25 @@ func TestControl_PoolQuery(t *testing.T) { }, }, }, - "query succeeds self_heal policies provided; missing pool self_heal property": { + "query succeeds; self_heal requested": { req: &PoolQueryReq{ - ID: poolUUID.String(), - QueryMask: daos.MustNewPoolQueryMask(daos.PoolQueryOptionSelfHealPolicy), + ID: poolUUID.String(), + Properties: []*daos.PoolProperty{propWithVal("self_heal", "")}, }, - // With SelfHealPolicy option selected, pool query gRPC is followed by pool - // get-prop self_heal which returns empty if no property has been set. mic: &MockInvokerConfig{ - UnaryResponseSet: []*UnaryResponse{ - MockMSResponse("host1", nil, queryResp(1)), - MockMSResponse("host1", nil, &mgmtpb.PoolGetPropResp{}), - }, + UnaryResponse: MockMSResponse("host1", nil, func() *mgmtpb.PoolQueryResp { + resp := queryResp(1) + resp.Properties = []*mgmtpb.PoolProperty{ + { + Number: propWithVal("self_heal", "").Number, + Value: &mgmtpb.PoolProperty_Numval{ + Numval: daos.PoolSelfHealingAutoExclude | + daos.PoolSelfHealingDelayRebuild, + }, + }, + } + return resp + }()), }, expResp: &PoolQueryResp{ PoolInfo: daos.PoolInfo{ @@ -2008,36 +1929,17 @@ func TestControl_PoolQuery(t *testing.T) { MediaType: daos.StorageMediaTypeNvme, }, }, - // GetPropResp returned without any matching self_heal - // property so default value applied. - SelfHealPolicy: "exclude;rebuild", + SelfHealPolicy: "exclude;delay_rebuild", }, SysSelfHealPolicy: "exclude;pool_exclude;pool_rebuild", }, }, - "query succeeds self_heal policies provided; pool self_heal property fetched": { + "query succeeds; self_heal not requested": { req: &PoolQueryReq{ - ID: poolUUID.String(), - QueryMask: daos.MustNewPoolQueryMask(daos.PoolQueryOptionSelfHealPolicy), + ID: poolUUID.String(), }, - // With SelfHealPolicy option selected, pool query gRPC is followed by pool - // get-prop self_heal which returns with delay_rebuild instead of rebuild - // flag. mic: &MockInvokerConfig{ - UnaryResponseSet: []*UnaryResponse{ - MockMSResponse("host1", nil, queryResp(1)), - MockMSResponse("host1", nil, &mgmtpb.PoolGetPropResp{ - Properties: []*mgmtpb.PoolProperty{ - { - Number: propWithVal("self_heal", "").Number, - Value: &mgmtpb.PoolProperty_Numval{ - daos.PoolSelfHealingAutoExclude | - daos.PoolSelfHealingDelayRebuild, - }, - }, - }, - }), - }, + UnaryResponse: MockMSResponse("host1", nil, queryResp(1)), }, expResp: &PoolQueryResp{ PoolInfo: daos.PoolInfo{ @@ -2069,50 +1971,10 @@ func TestControl_PoolQuery(t *testing.T) { MediaType: daos.StorageMediaTypeNvme, }, }, - // GetPropResp returned without any matching self_heal - // property so default value applied. - SelfHealPolicy: "exclude;delay_rebuild", }, SysSelfHealPolicy: "exclude;pool_exclude;pool_rebuild", }, }, - "pool get-prop returns error": { - req: &PoolQueryReq{ - ID: poolUUID.String(), - QueryMask: daos.MustNewPoolQueryMask(daos.PoolQueryOptionSelfHealPolicy), - }, - mic: &MockInvokerConfig{ - UnaryResponseSet: []*UnaryResponse{ - MockMSResponse("host1", nil, queryResp(1)), - MockMSResponse("host1", errors.New("get-prop failure"), nil), - }, - }, - expErr: errors.New("pool get-prop self_heal failed"), - }, - "pool get-prop returns multiple properties": { - req: &PoolQueryReq{ - ID: poolUUID.String(), - QueryMask: daos.MustNewPoolQueryMask(daos.PoolQueryOptionSelfHealPolicy), - }, - mic: &MockInvokerConfig{ - UnaryResponseSet: []*UnaryResponse{ - MockMSResponse("host1", nil, queryResp(1)), - MockMSResponse("host1", nil, &mgmtpb.PoolGetPropResp{ - Properties: []*mgmtpb.PoolProperty{ - { - Number: propWithVal("self_heal", "").Number, - Value: &mgmtpb.PoolProperty_Strval{Strval: "exclude"}, - }, - { - Number: propWithVal("self_heal", "").Number, - Value: &mgmtpb.PoolProperty_Strval{Strval: "rebuild"}, - }, - }, - }), - }, - }, - expErr: errors.New("> 1 occurrences of prop 4 in resp"), - }, "query with rebuild state busy with DER_OP_CANCELED (stopping)": { mic: &MockInvokerConfig{ UnaryResponse: MockMSResponse("host1", nil, @@ -2324,7 +2186,13 @@ func TestControl_PoolQuery(t *testing.T) { return } - if diff := cmp.Diff(tc.expResp, gotResp, rankSetCmpOpt()...); diff != "" { + cmpOpts := append(rankSetCmpOpt(), + cmpopts.IgnoreUnexported(daos.PoolProperty{}), + cmp.Comparer(func(a, b daos.PoolPropertyValue) bool { + return a.String() == b.String() + }), + ) + if diff := cmp.Diff(tc.expResp, gotResp, cmpOpts...); diff != "" { t.Fatalf("Unexpected response (-want, +got):\n%s\n", diff) } }) diff --git a/src/control/lib/control/response.go b/src/control/lib/control/response.go index a9a22dae1ed..29e90b1bfd2 100644 --- a/src/control/lib/control/response.go +++ b/src/control/lib/control/response.go @@ -1,5 +1,6 @@ // // (C) Copyright 2020-2023 Intel Corporation. +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -19,6 +20,7 @@ import ( "github.com/daos-stack/daos/src/control/common/proto/convert" ctlpb "github.com/daos-stack/daos/src/control/common/proto/ctl" "github.com/daos-stack/daos/src/control/lib/hostlist" + "github.com/daos-stack/daos/src/control/system" ) var ( @@ -210,11 +212,15 @@ func (ur *UnaryResponse) findMSResponse() (*HostResponse, error) { // As we may have sent the request to multiple MS replicas, just pick // through the responses to find ether the one that succeeded, or the one - // with the highest data version. - // If none succeeded, return the error from the last response. + // with the highest data version. If none succeeded, prefer a routing + // error, which says where to retry, over the error from the last response. var msResp *HostResponse var maxResp *HostResponse + var routeResp *HostResponse for _, msResp = range ur.Responses { + if msResp != nil && routeResp == nil && system.IsRoutingError(msResp.Error) { + routeResp = msResp + } if msResp == nil || msResp.Error != nil || msResp.Message == nil { continue } @@ -224,13 +230,15 @@ func (ur *UnaryResponse) findMSResponse() (*HostResponse, error) { } } - // If we found a response with a higher data version, then we - // know that it's a valid response and can bypass the rest of - // the error-checking logic. - if maxResp != nil && maxResp != msResp { + if maxResp != nil { return maxResp, nil } + if routeResp != nil { + ur.debugf("%s: err: %s", routeResp.Addr, routeResp.Error) + return nil, routeResp.Error + } + if msResp == nil { ur.debugf("no usable MS responses received") return nil, errNoMsResponse @@ -259,6 +267,9 @@ func (ur *UnaryResponse) getMSError() error { func (ur *UnaryResponse) getMSResponse() (proto.Message, error) { msr, err := ur.findMSResponse() if err != nil { + if IsConnErr(err) { + return nil, errors.Wrap(errMSConnectionFailure, err.Error()) + } return nil, err } @@ -272,9 +283,6 @@ func (ur *UnaryResponse) getMSResponse() (proto.Message, error) { func convertMSResponse(ur *UnaryResponse, out interface{}) error { msResp, err := ur.getMSResponse() if err != nil { - if IsConnErr(err) { - return errors.Wrap(errMSConnectionFailure, err.Error()) - } return err } diff --git a/src/control/lib/control/response_test.go b/src/control/lib/control/response_test.go index 61d3bbb1760..4d9737e3089 100644 --- a/src/control/lib/control/response_test.go +++ b/src/control/lib/control/response_test.go @@ -1,5 +1,6 @@ // // (C) Copyright 2020-2024 Intel Corporation. +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -19,6 +20,7 @@ import ( "github.com/daos-stack/daos/src/control/common/test" "github.com/daos-stack/daos/src/control/lib/hostlist" "github.com/daos-stack/daos/src/control/lib/ranklist" + "github.com/daos-stack/daos/src/control/system" ) func defResCmpOpts() []cmp.Option { @@ -142,6 +144,70 @@ func TestControl_getMSResponse(t *testing.T) { resp: MockMSResponse("host1", nil, nil), expErr: errors.New("message was nil"), }, + "all errors; last error returned": { + resp: &UnaryResponse{ + fromMS: true, + Responses: []*HostResponse{ + { + Addr: "host1", + Error: errors.New("first"), + }, + { + Addr: "host2", + Error: errors.New("last"), + }, + }, + }, + expErr: errors.New("last"), + }, + "all errors; routing error preferred": { + resp: &UnaryResponse{ + fromMS: true, + Responses: []*HostResponse{ + { + Addr: "host1", + Error: errors.New("first"), + }, + { + Addr: "host2", + Error: &system.ErrNotReplica{Replicas: []string{"host3"}}, + }, + { + Addr: "host3", + Error: errors.New("last"), + }, + }, + }, + expErr: &system.ErrNotReplica{Replicas: []string{"host3"}}, + }, + "routing error preferred over later nil message": { + resp: &UnaryResponse{ + fromMS: true, + Responses: []*HostResponse{ + { + Addr: "host1", + Error: &system.ErrNotReplica{Replicas: []string{"host3"}}, + }, + { + Addr: "host2", + }, + }, + }, + expErr: &system.ErrNotReplica{Replicas: []string{"host3"}}, + }, + "routing error preferred over later nil response": { + resp: &UnaryResponse{ + fromMS: true, + Responses: []*HostResponse{ + { + Addr: "host1", + Error: &system.ErrNotReplica{Replicas: []string{"host3"}}, + }, + nil, + }, + }, + expErr: &system.ErrNotReplica{Replicas: []string{"host3"}}, + }, "mixed versioned and non-versioned responses": { // NB: This is a weird "can't happen" situation, but // it shouldn't cause a panic. diff --git a/src/control/lib/control/rpc.go b/src/control/lib/control/rpc.go index e00374603c4..2e0ef9c6aa0 100644 --- a/src/control/lib/control/rpc.go +++ b/src/control/lib/control/rpc.go @@ -1,5 +1,6 @@ // // (C) Copyright 2020-2023 Intel Corporation. +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -377,9 +378,10 @@ func invokeUnaryRPC(parentCtx context.Context, log debugLogger, c UnaryInvoker, } } - // Copy the starting hostlist to use for reset on retry later. - startHostList := make([]string, len(req.getHostList())) - copy(startHostList, req.getHostList()) + // Where a retry restarts the leader search: the starting candidates + // until a routing error names the replicas. + searchHosts := make([]string, len(req.getHostList())) + copy(searchHosts, req.getHostList()) isHardFailure := func(err error, reqCtx context.Context) bool { if err == nil { @@ -461,9 +463,12 @@ func invokeUnaryRPC(parentCtx context.Context, log debugLogger, c UnaryInvoker, // send the retry. In the event that the hint was // empty (as can happen during an election), just send // the retry to all of the replicas. + if len(e.Replicas) > 0 { + searchHosts = e.Replicas + } if e.LeaderHint == "" { if len(e.Replicas) > 0 { - req.SetHostList(e.Replicas) + req.SetHostList(searchHosts) } break } @@ -474,7 +479,15 @@ func invokeUnaryRPC(parentCtx context.Context, log debugLogger, c UnaryInvoker, // One of them should be the current leader and will // service the request. if len(e.Replicas) > 0 { - req.SetHostList(e.Replicas) + searchHosts = e.Replicas + req.SetHostList(searchHosts) + } + case *system.ErrNoEngine: + // The replica has no engine to forward the request to; + // its peers may. + if len(e.Replicas) > 0 { + searchHosts = e.Replicas + req.SetHostList(searchHosts) } default: // As long as the outer context hasn't timed out, we @@ -492,9 +505,8 @@ func invokeUnaryRPC(parentCtx context.Context, log debugLogger, c UnaryInvoker, // is retryable, but doesn't define its own retry logic, // just break out so it can be tried again as usual. if req.canRetry(err, try) { - // Reset the hostlist to the starting hostlist, in order - // to restart the search for the current MS leader. - req.SetHostList(startHostList) + // Restart the search for the current MS leader. + req.SetHostList(searchHosts) break } diff --git a/src/control/lib/control/rpc_test.go b/src/control/lib/control/rpc_test.go index 2b9340b33b1..eed9e5641ae 100644 --- a/src/control/lib/control/rpc_test.go +++ b/src/control/lib/control/rpc_test.go @@ -1,5 +1,6 @@ // // (C) Copyright 2020-2024 Intel Corporation. +// (C) Copyright 2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -12,6 +13,7 @@ import ( "os" "runtime" "sort" + "sync/atomic" "testing" "time" @@ -467,6 +469,122 @@ func TestControl_InvokeUnaryRPC(t *testing.T) { }, expErr: errors.New("max retries"), }, + "routing error wins over a slower non-routing error": { + timeout: 5 * time.Second, + req: &testRequest{ + HostList: []string{"host00:10001", "host02:10001"}, + toMS: true, + rpcFn: func(_ context.Context, cc *grpc.ClientConn) (proto.Message, error) { + switch cc.Target() { + case "host00:10001": + return nil, errNotReplica + case "host02:10001": + time.Sleep(10 * time.Millisecond) + return nil, errors.New("engine not started") + } + return defaultMessage, nil + }, + }, + expResp: &UnaryResponse{ + Responses: []*HostResponse{ + { + Addr: "host01:10001", + Message: defaultMessage, + }, + { + Addr: "host05:10001", + Message: defaultMessage, + }, + { + Addr: "host07:10001", + Message: defaultMessage, + }, + { + Addr: "host08:10001", + Message: defaultMessage, + }, + { + Addr: "host09:10001", + Message: defaultMessage, + }, + }, + }, + }, + "request to replica without an engine is redirected to its peers": { + timeout: 5 * time.Second, + req: &testRequest{ + HostList: []string{"host01:10001"}, + toMS: true, + rpcFn: func(_ context.Context, cc *grpc.ClientConn) (proto.Message, error) { + if cc.Target() == "host01:10001" { + return nil, &system.ErrNoEngine{Replicas: nonLeaderReplicas[1:]} + } + return defaultMessage, nil + }, + }, + expResp: &UnaryResponse{ + Responses: []*HostResponse{ + { + Addr: "host05:10001", + Message: defaultMessage, + }, + { + Addr: "host07:10001", + Message: defaultMessage, + }, + { + Addr: "host09:10001", + Message: defaultMessage, + }, + }, + }, + }, + "retry after discovering replicas goes to the replicas": { + timeout: 5 * time.Second, + req: &testRequest{ + HostList: []string{"host00:10001"}, + toMS: true, + rpcFn: func() unaryRPC { + var nonReplicaCalls, replicaCalls atomic.Int32 + return func(_ context.Context, cc *grpc.ClientConn) (proto.Message, error) { + if cc.Target() == "host00:10001" { + if nonReplicaCalls.Add(1) > 1 { + return nil, errUnimplemented + } + return nil, errNotReplica + } + if replicaCalls.Add(1) <= int32(len(replicaHosts)) { + return nil, errors.New("transient") + } + return defaultMessage, nil + } + }(), + }, + expResp: &UnaryResponse{ + Responses: []*HostResponse{ + { + Addr: "host01:10001", + Message: defaultMessage, + }, + { + Addr: "host05:10001", + Message: defaultMessage, + }, + { + Addr: "host07:10001", + Message: defaultMessage, + }, + { + Addr: "host08:10001", + Message: defaultMessage, + }, + { + Addr: "host09:10001", + Message: defaultMessage, + }, + }, + }, + }, "request to non-replicas eventually discovers at least one replica": { req: &testRequest{ HostList: nonReplicaHosts, diff --git a/src/control/lib/daos/pool.go b/src/control/lib/daos/pool.go index f6d7835d948..e3e0ed7aa25 100644 --- a/src/control/lib/daos/pool.go +++ b/src/control/lib/daos/pool.go @@ -87,6 +87,7 @@ type ( MemFileBytes uint64 `json:"mem_file_bytes"` MdOnSsdActive bool `json:"md_on_ssd_active"` SelfHealPolicy string `json:"self_heal_policy"` + Properties []*PoolProperty `json:"properties,omitempty"` } // PoolQueryTargetState represents the current state of the pool target. @@ -119,9 +120,9 @@ type ( const ( // DefaultPoolQueryMask defines the default pool query mask. - DefaultPoolQueryMask = PoolQueryMask(^uint64(0) &^ (C.DPI_ENGINES_ENABLED | C.DPI_ENGINES_DEAD | C.DPI_SELF_HEAL_POLICY)) + DefaultPoolQueryMask = PoolQueryMask(^uint64(0) &^ (C.DPI_ENGINES_ENABLED | C.DPI_ENGINES_DEAD)) // HealthOnlyPoolQueryMask defines the mask for health-only queries. - HealthOnlyPoolQueryMask = PoolQueryMask(^uint64(0) &^ (C.DPI_ENGINES_ENABLED | C.DPI_SPACE | C.DPI_SELF_HEAL_POLICY)) + HealthOnlyPoolQueryMask = PoolQueryMask(^uint64(0) &^ (C.DPI_ENGINES_ENABLED | C.DPI_SPACE)) // PoolQueryOptionSpace retrieves storage space usage as part of the pool query. PoolQueryOptionSpace PoolQueryOption = "space" @@ -133,8 +134,6 @@ const ( PoolQueryOptionDisabledEngines PoolQueryOption = "disabled_engines" // PoolQueryOptionDeadEngines retrieves dead engines as part of the pool query. PoolQueryOptionDeadEngines PoolQueryOption = "dead_engines" - // PoolQueryOptionSelfHealPolicy retrieves self_heal_policy in addition to the pool query. - PoolQueryOptionSelfHealPolicy PoolQueryOption = "self_heal_policy" // PoolConnectFlagReadOnly indicates that the connection is read-only. PoolConnectFlagReadOnly PoolConnectFlag = C.DAOS_PC_RO @@ -169,7 +168,6 @@ var poolQueryOptMap = map[C.int]PoolQueryOption{ C.DPI_ENGINES_ENABLED: PoolQueryOptionEnabledEngines, C.DPI_ENGINES_DISABLED: PoolQueryOptionDisabledEngines, C.DPI_ENGINES_DEAD: PoolQueryOptionDeadEngines, - C.DPI_SELF_HEAL_POLICY: PoolQueryOptionSelfHealPolicy, } func resolvePoolQueryOpt(name PoolQueryOption) (C.int, error) { diff --git a/src/control/lib/daos/pool_test.go b/src/control/lib/daos/pool_test.go index 5c90e03784d..443bd88c84b 100644 --- a/src/control/lib/daos/pool_test.go +++ b/src/control/lib/daos/pool_test.go @@ -150,8 +150,7 @@ func TestDaos_PoolQueryMask(t *testing.T) { pqm.SetAll() }), expString: genOptsStr(PoolQueryOptionDeadEngines, PoolQueryOptionDisabledEngines, - PoolQueryOptionEnabledEngines, PoolQueryOptionRebuild, - PoolQueryOptionSelfHealPolicy, PoolQueryOptionSpace), + PoolQueryOptionEnabledEngines, PoolQueryOptionRebuild, PoolQueryOptionSpace), }, "set query all=false": { testMask: genTestMask(func(pqm *PoolQueryMask) { @@ -172,8 +171,7 @@ func TestDaos_PoolQueryMask(t *testing.T) { pqm.ClearOptions(PoolQueryOptionSpace) }), expString: genOptsStr(PoolQueryOptionDeadEngines, PoolQueryOptionDisabledEngines, - PoolQueryOptionEnabledEngines, PoolQueryOptionRebuild, - PoolQueryOptionSelfHealPolicy), + PoolQueryOptionEnabledEngines, PoolQueryOptionRebuild), }, "set query space=false (already false)": { testMask: genTestMask(func(pqm *PoolQueryMask) { @@ -193,7 +191,7 @@ func TestDaos_PoolQueryMask(t *testing.T) { pqm.ClearOptions(PoolQueryOptionRebuild) }), expString: genOptsStr(PoolQueryOptionDeadEngines, PoolQueryOptionDisabledEngines, - PoolQueryOptionEnabledEngines, PoolQueryOptionSelfHealPolicy, + PoolQueryOptionEnabledEngines, PoolQueryOptionSpace), }, "set query enabled_engines=true": { @@ -208,7 +206,7 @@ func TestDaos_PoolQueryMask(t *testing.T) { pqm.ClearOptions(PoolQueryOptionEnabledEngines) }), expString: genOptsStr(PoolQueryOptionDeadEngines, PoolQueryOptionDisabledEngines, - PoolQueryOptionRebuild, PoolQueryOptionSelfHealPolicy, + PoolQueryOptionRebuild, PoolQueryOptionSpace), }, "set query disabled_engines=true": { @@ -223,7 +221,7 @@ func TestDaos_PoolQueryMask(t *testing.T) { pqm.ClearOptions(PoolQueryOptionDisabledEngines) }), expString: genOptsStr(PoolQueryOptionDeadEngines, PoolQueryOptionEnabledEngines, - PoolQueryOptionRebuild, PoolQueryOptionSelfHealPolicy, + PoolQueryOptionRebuild, PoolQueryOptionSpace), }, } { @@ -276,7 +274,7 @@ func TestDaos_PoolQueryMaskMarshalJSON(t *testing.T) { testMask: genTestMask(func(pqm *PoolQueryMask) { pqm.SetAll() }), - expJSON: []byte(`"dead_engines,disabled_engines,enabled_engines,rebuild,self_heal_policy,space"`), + expJSON: []byte(`"dead_engines,disabled_engines,enabled_engines,rebuild,space"`), }, } { t.Run(name, func(t *testing.T) { @@ -306,7 +304,7 @@ func TestDaos_PoolQueryMaskUnmarshalJSON(t *testing.T) { }, "uint64 value": { testData: []byte("18446744073709551603"), - expString: "dead_engines,rebuild,self_heal_policy,space", + expString: "dead_engines,rebuild,space", }, "string values": { testData: []byte("rebuild,disabled_engines"), diff --git a/src/control/server/harness.go b/src/control/server/harness.go index c35223466cc..c01fd689310 100644 --- a/src/control/server/harness.go +++ b/src/control/server/harness.go @@ -273,6 +273,19 @@ func (h *EngineHarness) Start(ctx context.Context, db dbLeader, cfg *config.Serv return ctx.Err() } +// hasReadyEngine returns true if at least one instance can service dRPC calls. +func (h *EngineHarness) hasReadyEngine() bool { + h.RLock() + defer h.RUnlock() + + for _, ei := range h.instances { + if ei.IsReady() { + return true + } + } + return false +} + // readyRanks returns rank assignment of configured harness instances that are // in a ready state. Rank assignments can be nil. func (h *EngineHarness) readyRanks() []ranklist.Rank { diff --git a/src/control/server/interceptors.go b/src/control/server/interceptors.go index db116fb95fa..d223694627b 100644 --- a/src/control/server/interceptors.go +++ b/src/control/server/interceptors.go @@ -1,6 +1,6 @@ // // (C) Copyright 2019-2023 Intel Corporation. -// (C) Copyright 2025 Hewlett Packard Enterprise Development LP +// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -248,7 +248,7 @@ func unaryStatusInterceptor(ctx context.Context, req interface{}, info *grpc.Una // isSentinelErr indicates whether or not the error is a sentinel // error used to convey a specific state to the client. func isSentinelErr(err error) bool { - return system.IsNotReady(err) || system.IsNotReplica(err) || system.IsNotLeader(err) + return system.IsNotReady(err) || system.IsRoutingError(err) } // shouldLogMsg determines whether or not the given message should be logged. diff --git a/src/control/server/mgmt_cont.go b/src/control/server/mgmt_cont.go index 70308ee0173..a02f754808c 100644 --- a/src/control/server/mgmt_cont.go +++ b/src/control/server/mgmt_cont.go @@ -1,6 +1,6 @@ // // (C) Copyright 2020-2024 Intel Corporation. -// (C) Copyright 2025 Hewlett Packard Enterprise Development LP +// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -19,7 +19,7 @@ import ( // ListContainers forwards a gRPC request to the DAOS I/O Engine to retrieve a pool's // list of containers. func (svc *mgmtSvc) ListContainers(ctx context.Context, req *mgmtpb.ListContReq) (*mgmtpb.ListContResp, error) { - if err := svc.checkReplicaRequest(req); err != nil { + if err := svc.checkForwardedReplicaRequest(req); err != nil { return nil, err } @@ -38,7 +38,7 @@ func (svc *mgmtSvc) ListContainers(ctx context.Context, req *mgmtpb.ListContReq) // ContSetOwner forwards a gRPC request to the DAOS I/O Engine to change a container's ownership. func (svc *mgmtSvc) ContSetOwner(ctx context.Context, req *mgmtpb.ContSetOwnerReq) (*mgmtpb.DaosResp, error) { - if err := svc.checkReplicaRequest(req); err != nil { + if err := svc.checkForwardedReplicaRequest(req); err != nil { return nil, err } diff --git a/src/control/server/mgmt_cont_test.go b/src/control/server/mgmt_cont_test.go index cedc85d4926..c0e3eaa7ee8 100644 --- a/src/control/server/mgmt_cont_test.go +++ b/src/control/server/mgmt_cont_test.go @@ -89,7 +89,7 @@ func TestMgmt_ListContainers(t *testing.T) { events.NewPubSub(test.Context(t), log)) }, req: validListContReq(), - expErr: FaultHarnessNotStarted, + expErr: &system.ErrNoEngine{}, }, "drpc error": { setupDrpc: func(t *testing.T, svc *mgmtSvc) { @@ -192,7 +192,7 @@ func TestMgmt_ContSetOwner(t *testing.T) { events.NewPubSub(test.Context(t), log)) }, req: validContSetOwnerReq(), - expErr: FaultHarnessNotStarted, + expErr: &system.ErrNoEngine{}, }, "drpc error": { setupDrpc: func(t *testing.T, svc *mgmtSvc) { diff --git a/src/control/server/mgmt_pool.go b/src/control/server/mgmt_pool.go index 5b8984b2bcf..79d7e7e3141 100644 --- a/src/control/server/mgmt_pool.go +++ b/src/control/server/mgmt_pool.go @@ -29,13 +29,6 @@ import ( "github.com/daos-stack/daos/src/control/system" ) -/* -#include - -#include -*/ -import "C" - const ( // DefaultPoolScmRatio defines the default SCM:NVMe ratio for // requests that do not specify one. @@ -1075,7 +1068,7 @@ func (svc *mgmtSvc) PoolReintegrate(ctx context.Context, req *mgmtpb.PoolReintRe // PoolQuery forwards a pool query request to the I/O Engine. func (svc *mgmtSvc) PoolQuery(ctx context.Context, req *mgmtpb.PoolQueryReq) (*mgmtpb.PoolQueryResp, error) { - if err := svc.checkReplicaRequest(req); err != nil { + if err := svc.checkForwardedReplicaRequest(req); err != nil { return nil, err } @@ -1096,18 +1089,15 @@ func (svc *mgmtSvc) PoolQuery(ctx context.Context, req *mgmtpb.PoolQueryReq) (*m // Preserve compatibility with pre-2.6 callers. resp.Leader = resp.SvcLdr - // Retrieve system self-heal property. Assume default value where all flags are set if - // property isn't present. + // Assume the default system self-heal policy if the property isn't set. resp.SysSelfHealPolicy = daos.DefaultSysSelfHealFlagsStr - if req.QueryMask&C.DPI_SELF_HEAL_POLICY != 0 { - if selfHeal, err := svc.getSysSelfHeal(); system.IsErrSystemAttrNotFound(err) { - svc.log.Debugf(err.Error()) - } else if err != nil { - return nil, err - } else { - svc.log.Debugf("system self-heal: %s", selfHeal) - resp.SysSelfHealPolicy = selfHeal - } + if selfHeal, err := svc.getSysSelfHeal(); system.IsErrSystemAttrNotFound(err) { + svc.log.Debugf(err.Error()) + } else if err != nil { + return nil, err + } else { + svc.log.Debugf("system self-heal: %s", selfHeal) + resp.SysSelfHealPolicy = selfHeal } return resp, nil @@ -1115,7 +1105,7 @@ func (svc *mgmtSvc) PoolQuery(ctx context.Context, req *mgmtpb.PoolQueryReq) (*m // PoolQueryTarget forwards a pool query targets request to the I/O Engine. func (svc *mgmtSvc) PoolQueryTarget(ctx context.Context, req *mgmtpb.PoolQueryTargetReq) (*mgmtpb.PoolQueryTargetResp, error) { - if err := svc.checkReplicaRequest(req); err != nil { + if err := svc.checkForwardedReplicaRequest(req); err != nil { return nil, err } @@ -1272,7 +1262,7 @@ func (svc *mgmtSvc) PoolSetProp(parent context.Context, req *mgmtpb.PoolSetPropR // PoolGetProp forwards a request to the I/O Engine to get pool properties. func (svc *mgmtSvc) PoolGetProp(ctx context.Context, req *mgmtpb.PoolGetPropReq) (*mgmtpb.PoolGetPropResp, error) { - if err := svc.checkReplicaRequest(req); err != nil { + if err := svc.checkForwardedReplicaRequest(req); err != nil { return nil, err } @@ -1302,7 +1292,7 @@ func (svc *mgmtSvc) PoolGetProp(ctx context.Context, req *mgmtpb.PoolGetPropReq) // PoolGetACL forwards a request to the I/O Engine to fetch a pool's Access Control List func (svc *mgmtSvc) PoolGetACL(ctx context.Context, req *mgmtpb.GetACLReq) (*mgmtpb.ACLResp, error) { - if err := svc.checkReplicaRequest(req); err != nil { + if err := svc.checkForwardedReplicaRequest(req); err != nil { return nil, err } diff --git a/src/control/server/mgmt_pool_test.go b/src/control/server/mgmt_pool_test.go index 90ba85d074a..6c9227ecb91 100644 --- a/src/control/server/mgmt_pool_test.go +++ b/src/control/server/mgmt_pool_test.go @@ -2510,6 +2510,9 @@ func TestServer_MgmtSvc_PoolQuery(t *testing.T) { missingSB := newTestMgmtSvc(t, log) missingSB.harness.instances[0].(*EngineInstance)._superblock = nil + noEngine := newTestMgmtSvc(t, log) + noEngine.harness.instances[0].(*EngineInstance).ready.SetFalse() + allRanksDown := newTestMgmtSvc(t, log) downRanksPool := test.MockUUID(9) addTestPools(t, allRanksDown.sysdb, downRanksPool) @@ -2569,6 +2572,13 @@ func TestServer_MgmtSvc_PoolQuery(t *testing.T) { }, expErr: errors.New("available"), }, + "no running engine": { + mgmtSvc: noEngine, + req: &mgmtpb.PoolQueryReq{ + Id: mockUUID, + }, + expErr: &system.ErrNoEngine{}, + }, "successful query": { req: &mgmtpb.PoolQueryReq{ Id: mockUUID, @@ -2576,7 +2586,7 @@ func TestServer_MgmtSvc_PoolQuery(t *testing.T) { expResp: &mgmtpb.PoolQueryResp{ State: mgmtpb.PoolServiceState_Ready, Uuid: mockUUID, - SysSelfHealPolicy: daos.DefaultSysSelfHealFlagsStr, + SysSelfHealPolicy: "pool_rebuild", }, }, "successful query (includes pre-2.6 Leader field)": { @@ -2596,7 +2606,7 @@ func TestServer_MgmtSvc_PoolQuery(t *testing.T) { Uuid: mockUUID, SvcLdr: 42, Leader: 42, - SysSelfHealPolicy: daos.DefaultSysSelfHealFlagsStr, + SysSelfHealPolicy: "pool_rebuild", }, }, "successful query; mdonssd enabled": { @@ -2615,25 +2625,13 @@ func TestServer_MgmtSvc_PoolQuery(t *testing.T) { State: mgmtpb.PoolServiceState_Ready, Uuid: mockUUID, MemFileBytes: humanize.GiByte, - SysSelfHealPolicy: daos.DefaultSysSelfHealFlagsStr, - }, - }, - "successful query; sys self-heal prop fetch": { - req: &mgmtpb.PoolQueryReq{ - Id: mockUUID, - QueryMask: uint64(daos.MustNewPoolQueryMask(daos.PoolQueryOptionSelfHealPolicy)), - }, - expResp: &mgmtpb.PoolQueryResp{ - State: mgmtpb.PoolServiceState_Ready, - Uuid: mockUUID, SysSelfHealPolicy: "pool_rebuild", }, }, - "successful query; sys self-heal prop fetch; missing system property": { + "successful query; missing system self-heal property": { missingSelfHealSysProp: true, req: &mgmtpb.PoolQueryReq{ - Id: mockUUID, - QueryMask: uint64(daos.MustNewPoolQueryMask(daos.PoolQueryOptionSelfHealPolicy)), + Id: mockUUID, }, expResp: &mgmtpb.PoolQueryResp{ State: mgmtpb.PoolServiceState_Ready, diff --git a/src/control/server/mgmt_svc.go b/src/control/server/mgmt_svc.go index 2dfd8f3449f..5b4d22d3487 100644 --- a/src/control/server/mgmt_svc.go +++ b/src/control/server/mgmt_svc.go @@ -202,6 +202,20 @@ func (svc *mgmtSvc) checkReplicaRequest(req proto.Message) error { return svc.sysdb.CheckReplica() } +// checkForwardedReplicaRequest performs sanity-checking on a request that must +// be run on a MS replica and forwarded to one of its engines. A replica without +// a running engine redirects the caller to its peers instead of failing. +func (svc *mgmtSvc) checkForwardedReplicaRequest(req proto.Message) error { + if err := svc.checkReplicaRequest(req); err != nil { + return err + } + + if svc.harness.hasReadyEngine() { + return nil + } + return &system.ErrNoEngine{Replicas: svc.sysdb.PeerReplicas()} +} + // startLeaderLoops kicks off the leader-only processing loops // that will be canceled on leadership loss. func (svc *mgmtSvc) startLeaderLoops(ctx context.Context) { diff --git a/src/control/system/errors.go b/src/control/system/errors.go index 5a3294efe14..3bf70f9879b 100644 --- a/src/control/system/errors.go +++ b/src/control/system/errors.go @@ -83,6 +83,24 @@ func IsNotReplica(err error) bool { return ok } +// ErrNoEngine indicates that a request was made to a Management +// Service replica that has no running engine to forward it to. +type ErrNoEngine struct { + Replicas []string +} + +func (err *ErrNoEngine) Error() string { + return fmt.Sprintf("no running engine on this %s replica (try one of %s)", + build.ManagementServiceName, strings.Join(err.Replicas, ",")) +} + +// IsNoEngine returns a boolean indicating whether or not the +// supplied error is an instance of ErrNoEngine. +func IsNoEngine(err error) bool { + _, ok := errors.Cause(err).(*ErrNoEngine) + return ok +} + // ErrNotLeader indicates that a request was made to a control plane // instance that is not the current Management Service Leader. type ErrNotLeader struct { @@ -102,6 +120,12 @@ func IsNotLeader(err error) bool { return ok } +// IsRoutingError returns a boolean indicating whether or not the +// supplied error tells the caller which hosts to retry. +func IsRoutingError(err error) bool { + return IsNotReplica(err) || IsNotLeader(err) || IsNoEngine(err) +} + // ErrMemberExists indicates the failure of an operation that // expected the given member to not exist. type ErrMemberExists struct { diff --git a/src/control/system/raft/database.go b/src/control/system/raft/database.go index 00e8618abee..940fe75462c 100644 --- a/src/control/system/raft/database.go +++ b/src/control/system/raft/database.go @@ -338,6 +338,11 @@ func (db *Database) PeerAddrs() ([]*net.TCPAddr, error) { return peers, nil } +// PeerReplicas returns the addresses of this system's replication peers. +func (db *Database) PeerReplicas() []string { + return db.cfg.stringReplicas(db.replicaAddr) +} + // IsReplica returns true if the system is configured as a replica. func (db *Database) IsReplica() bool { return db != nil && db.replicaAddr != nil diff --git a/src/include/daos_pool.h b/src/include/daos_pool.h index 35c018c93c6..7904d60065f 100644 --- a/src/include/daos_pool.h +++ b/src/include/daos_pool.h @@ -164,8 +164,6 @@ enum daos_pool_info_bit { DPI_ENGINES_DEAD = 1ULL << 4, /** true to query pool's maximum supported rebuild layout version */ DPI_REBUILD_MAX_LAYOUT_VER = 1ULL << 5, - /** true to query pool's self_heal policy. currently fetch implemented in control plane */ - DPI_SELF_HEAL_POLICY = 1ULL << 6, /** query all above optional info */ DPI_ALL = -1, }; diff --git a/src/include/daos_srv/pool.h b/src/include/daos_srv/pool.h index f2161183461..50d250b3974 100644 --- a/src/include/daos_srv/pool.h +++ b/src/include/daos_srv/pool.h @@ -318,9 +318,10 @@ int dsc_pool_svc_delete_acl(uuid_t pool_uuid, d_rank_list_t *ranks, uint64_t dea enum daos_acl_principal_type principal_type, const char *principal_name); -int dsc_pool_svc_query(uuid_t pool_uuid, d_rank_list_t *ps_ranks, uint64_t deadline, +int + dsc_pool_svc_query(uuid_t pool_uuid, d_rank_list_t *ps_ranks, uint64_t deadline, d_rank_list_t **enabled_ranks, d_rank_list_t **disabled_ranks, - d_rank_list_t **dead_ranks, daos_pool_info_t *pool_info, + d_rank_list_t **dead_ranks, daos_pool_info_t *pool_info, daos_prop_t *prop, uint32_t *pool_layout_ver, uint32_t *upgrade_layout_ver, uint64_t *mem_file_bytes); int dsc_pool_svc_query_target(uuid_t pool_uuid, d_rank_list_t *ps_ranks, uint64_t deadline, diff --git a/src/mgmt/pool.pb-c.c b/src/mgmt/pool.pb-c.c index 36df9d536a2..5f3be16db81 100644 --- a/src/mgmt/pool.pb-c.c +++ b/src/mgmt/pool.pb-c.c @@ -3610,7 +3610,7 @@ const ProtobufCMessageDescriptor mgmt__list_cont_resp__descriptor = (ProtobufCMessageInit) mgmt__list_cont_resp__init, NULL,NULL,NULL /* reserved[123] */ }; -static const ProtobufCFieldDescriptor mgmt__pool_query_req__field_descriptors[4] = +static const ProtobufCFieldDescriptor mgmt__pool_query_req__field_descriptors[5] = { { "sys", @@ -3660,9 +3660,22 @@ static const ProtobufCFieldDescriptor mgmt__pool_query_req__field_descriptors[4] 0, /* flags */ 0,NULL,NULL /* reserved1,reserved2, etc */ }, + { + "properties", + 5, + PROTOBUF_C_LABEL_REPEATED, + PROTOBUF_C_TYPE_MESSAGE, + offsetof(Mgmt__PoolQueryReq, n_properties), + offsetof(Mgmt__PoolQueryReq, properties), + &mgmt__pool_property__descriptor, + NULL, + 0, /* flags */ + 0,NULL,NULL /* reserved1,reserved2, etc */ + }, }; static const unsigned mgmt__pool_query_req__field_indices_by_name[] = { 1, /* field[1] = id */ + 4, /* field[4] = properties */ 3, /* field[3] = query_mask */ 2, /* field[2] = svc_ranks */ 0, /* field[0] = sys */ @@ -3670,7 +3683,7 @@ static const unsigned mgmt__pool_query_req__field_indices_by_name[] = { static const ProtobufCIntRange mgmt__pool_query_req__number_ranges[1 + 1] = { { 1, 0 }, - { 0, 4 } + { 0, 5 } }; const ProtobufCMessageDescriptor mgmt__pool_query_req__descriptor = { @@ -3680,7 +3693,7 @@ const ProtobufCMessageDescriptor mgmt__pool_query_req__descriptor = "Mgmt__PoolQueryReq", "mgmt", sizeof(Mgmt__PoolQueryReq), - 4, + 5, mgmt__pool_query_req__field_descriptors, mgmt__pool_query_req__field_indices_by_name, 1, mgmt__pool_query_req__number_ranges, @@ -4198,14 +4211,14 @@ static const ProtobufCFieldDescriptor mgmt__pool_query_resp__field_descriptors[2 0,NULL,NULL /* reserved1,reserved2, etc */ }, { - "self_heal_policy", + "properties", 24, - PROTOBUF_C_LABEL_NONE, - PROTOBUF_C_TYPE_STRING, - 0, /* quantifier_offset */ - offsetof(Mgmt__PoolQueryResp, self_heal_policy), + PROTOBUF_C_LABEL_REPEATED, + PROTOBUF_C_TYPE_MESSAGE, + offsetof(Mgmt__PoolQueryResp, n_properties), + offsetof(Mgmt__PoolQueryResp, properties), + &mgmt__pool_property__descriptor, NULL, - &protobuf_c_empty_string, 0, /* flags */ 0,NULL,NULL /* reserved1,reserved2, etc */ }, @@ -4233,9 +4246,9 @@ static const unsigned mgmt__pool_query_resp__field_indices_by_name[] = { 21, /* field[21] = md_on_ssd_active */ 19, /* field[19] = mem_file_bytes */ 13, /* field[13] = pool_layout_ver */ + 22, /* field[22] = properties */ 18, /* field[18] = query_mask */ 6, /* field[6] = rebuild */ - 22, /* field[22] = self_heal_policy */ 15, /* field[15] = state */ 0, /* field[0] = status */ 16, /* field[16] = svc_ldr */ diff --git a/src/mgmt/pool.pb-c.h b/src/mgmt/pool.pb-c.h index a99de719603..d6da7b7cbd4 100644 --- a/src/mgmt/pool.pb-c.h +++ b/src/mgmt/pool.pb-c.h @@ -771,10 +771,15 @@ struct _Mgmt__PoolQueryReq * Bitmask of pool query options */ uint64_t query_mask; + /* + * Pool properties to return with the query + */ + size_t n_properties; + Mgmt__PoolProperty **properties; }; #define MGMT__POOL_QUERY_REQ__INIT \ { PROTOBUF_C_MESSAGE_INIT (&mgmt__pool_query_req__descriptor) \ - , (char *)protobuf_c_empty_string, (char *)protobuf_c_empty_string, 0,NULL, 0 } + , (char *)protobuf_c_empty_string, (char *)protobuf_c_empty_string, 0,NULL, 0, 0,NULL } /* @@ -917,9 +922,10 @@ struct _Mgmt__PoolQueryResp */ protobuf_c_boolean md_on_ssd_active; /* - * pool property self_heal value + * requested pool properties */ - char *self_heal_policy; + size_t n_properties; + Mgmt__PoolProperty **properties; /* * system property self_heal value */ @@ -927,7 +933,7 @@ struct _Mgmt__PoolQueryResp }; #define MGMT__POOL_QUERY_RESP__INIT \ { PROTOBUF_C_MESSAGE_INIT (&mgmt__pool_query_resp__descriptor) \ - , 0, (char *)protobuf_c_empty_string, (char *)protobuf_c_empty_string, 0, 0, 0, NULL, 0,NULL, 0, 0, (char *)protobuf_c_empty_string, (char *)protobuf_c_empty_string, 0, 0, 0, MGMT__POOL_SERVICE_STATE__Creating, 0, 0,NULL, 0, 0, (char *)protobuf_c_empty_string, 0, (char *)protobuf_c_empty_string, (char *)protobuf_c_empty_string } + , 0, (char *)protobuf_c_empty_string, (char *)protobuf_c_empty_string, 0, 0, 0, NULL, 0,NULL, 0, 0, (char *)protobuf_c_empty_string, (char *)protobuf_c_empty_string, 0, 0, 0, MGMT__POOL_SERVICE_STATE__Creating, 0, 0,NULL, 0, 0, (char *)protobuf_c_empty_string, 0, 0,NULL, (char *)protobuf_c_empty_string } typedef enum { diff --git a/src/mgmt/srv_drpc.c b/src/mgmt/srv_drpc.c index f78223cdce6..48246c01028 100644 --- a/src/mgmt/srv_drpc.c +++ b/src/mgmt/srv_drpc.c @@ -414,7 +414,7 @@ static int pool_create_fill_resp(Mgmt__PoolCreateResp *resp, uuid_t uuid, d_rank rc = ds_mgmt_pool_query(uuid, svc_ranks, &enabled_ranks, NULL, NULL, daos_getmtime_coarse() + 2 * 60 * 1000, &pool_info, NULL, NULL, - &mem_file_bytes); + NULL, &mem_file_bytes); if (DAOS_FAIL_CHECK(DAOS_MGMT_FAIL_CREATE_QUERY)) rc = -DER_TIMEDOUT; if (rc != 0) { @@ -1174,7 +1174,7 @@ free_response_props(Mgmt__PoolProperty **props, size_t n_props) } static int -add_props_to_resp(daos_prop_t *prop, Mgmt__PoolGetPropResp *resp) +add_props_to_resp(daos_prop_t *prop, Mgmt__PoolProperty ***out_props, size_t *out_n_props) { Mgmt__PoolProperty **resp_props; struct daos_prop_entry *entry; @@ -1261,8 +1261,8 @@ add_props_to_resp(daos_prop_t *prop, Mgmt__PoolGetPropResp *resp) j++; } - resp->properties = resp_props; - resp->n_properties = valid_prop_nr; + *out_props = resp_props; + *out_n_props = valid_prop_nr; out: if (rc != 0) @@ -1317,7 +1317,7 @@ void ds_mgmt_drpc_pool_get_prop(Drpc__Call *drpc_req, Drpc__Response *drpc_resp) goto out_ranks; } - rc = add_props_to_resp(props, &resp); + rc = add_props_to_resp(props, &resp.properties, &resp.n_properties); if (rc != 0) goto out_ranks; @@ -1822,6 +1822,7 @@ ds_mgmt_drpc_pool_query(Drpc__Call *drpc_req, Drpc__Response *drpc_resp) Mgmt__PoolRebuildStatus rebuild = MGMT__POOL_REBUILD_STATUS__INIT; uuid_t uuid; daos_pool_info_t pool_info = {0}; + daos_prop_t *prop = NULL; d_rank_list_t *svc_ranks = NULL; d_rank_list_t *enabled_ranks = NULL; d_rank_list_t *disabled_ranks = NULL; @@ -1851,15 +1852,25 @@ ds_mgmt_drpc_pool_query(Drpc__Call *drpc_req, Drpc__Response *drpc_resp) if (svc_ranks == NULL) D_GOTO(error, rc = -DER_NOMEM); + if (req->n_properties > 0) { + rc = conv_req_props(&prop, false, req->properties, req->n_properties); + if (rc != 0) + D_GOTO(error, rc); + } + pool_info.pi_bits = req->query_mask; rc = ds_mgmt_pool_query(uuid, svc_ranks, &enabled_ranks, &disabled_ranks, &dead_ranks, - mgmt_ps_call_deadline(), &pool_info, &resp.pool_layout_ver, + mgmt_ps_call_deadline(), &pool_info, prop, &resp.pool_layout_ver, &resp.upgrade_layout_ver, &resp.mem_file_bytes); if (rc != 0) { DL_ERROR(rc, DF_UUID ": Failed to query the pool", DP_UUID(uuid)); D_GOTO(error, rc); } + rc = add_props_to_resp(prop, &resp.properties, &resp.n_properties); + if (rc != 0) + D_GOTO(error, rc); + rc = d_rank_list_to_str(enabled_ranks, &enabled_ranks_str); if (rc != 0) { DL_ERROR(rc, DF_UUID ": Failed to serialize the list of enabled ranks", @@ -1939,6 +1950,8 @@ ds_mgmt_drpc_pool_query(Drpc__Call *drpc_req, Drpc__Response *drpc_resp) mgmt__pool_query_req__free_unpacked(req, &alloc.alloc); + free_response_props(resp.properties, resp.n_properties); + daos_prop_free(prop); d_rank_list_free(enabled_ranks); D_FREE(enabled_ranks_str); d_rank_list_free(disabled_ranks); diff --git a/src/mgmt/srv_internal.h b/src/mgmt/srv_internal.h index d22a505ec65..a6f9997c359 100644 --- a/src/mgmt/srv_internal.h +++ b/src/mgmt/srv_internal.h @@ -128,7 +128,7 @@ int ds_mgmt_pool_list_cont(uuid_t uuid, d_rank_list_t *svc_ranks, int ds_mgmt_pool_query(uuid_t pool_uuid, d_rank_list_t *svc_ranks, d_rank_list_t **enabled_ranks, d_rank_list_t **disabled_ranks, d_rank_list_t **dead_ranks, uint64_t deadline, - daos_pool_info_t *pool_info, uint32_t *pool_layout_ver, + daos_pool_info_t *pool_info, daos_prop_t *prop, uint32_t *pool_layout_ver, uint32_t *upgrade_layout_ver, uint64_t *mem_file_bytes); int ds_mgmt_pool_query_targets(uuid_t pool_uuid, d_rank_list_t *svc_ranks, d_rank_t rank, diff --git a/src/mgmt/srv_pool.c b/src/mgmt/srv_pool.c index 18d4f154ea3..44751c38fc7 100644 --- a/src/mgmt/srv_pool.c +++ b/src/mgmt/srv_pool.c @@ -503,6 +503,7 @@ ds_mgmt_pool_list_cont(uuid_t uuid, d_rank_list_t *svc_ranks, * \param[out] dead_ranks Optional, returned storage ranks marked DEAD by SWIM. * \param[in] deadline Unix time deadline in milliseconds * \param[in][out] pool_info Query results + * \param[in][out] prop Optional, properties to query * \param[in][out] pool_layout_ver Pool global version * \param[in][out] upgrade_layout_ver Latest pool global version this pool might be upgraded * @@ -513,7 +514,7 @@ ds_mgmt_pool_list_cont(uuid_t uuid, d_rank_list_t *svc_ranks, int ds_mgmt_pool_query(uuid_t pool_uuid, d_rank_list_t *svc_ranks, d_rank_list_t **enabled_ranks, d_rank_list_t **disabled_ranks, d_rank_list_t **dead_ranks, uint64_t deadline, - daos_pool_info_t *pool_info, uint32_t *pool_layout_ver, + daos_pool_info_t *pool_info, daos_prop_t *prop, uint32_t *pool_layout_ver, uint32_t *upgrade_layout_ver, uint64_t *mem_file_bytes) { if (pool_info == NULL) { @@ -524,7 +525,7 @@ ds_mgmt_pool_query(uuid_t pool_uuid, d_rank_list_t *svc_ranks, d_rank_list_t **e D_DEBUG(DB_MGMT, "Querying pool "DF_UUID"\n", DP_UUID(pool_uuid)); return dsc_pool_svc_query(pool_uuid, svc_ranks, deadline, enabled_ranks, disabled_ranks, - dead_ranks, pool_info, pool_layout_ver, upgrade_layout_ver, + dead_ranks, pool_info, prop, pool_layout_ver, upgrade_layout_ver, mem_file_bytes); } diff --git a/src/mgmt/tests/mocks.c b/src/mgmt/tests/mocks.c index 0c24b1fb748..09c09f07c5a 100644 --- a/src/mgmt/tests/mocks.c +++ b/src/mgmt/tests/mocks.c @@ -284,11 +284,13 @@ void *ds_mgmt_pool_query_info_ptr; d_rank_list_t *ds_mgmt_pool_query_enabled_ranks_out; d_rank_list_t *ds_mgmt_pool_query_disabled_ranks_out; d_rank_list_t *ds_mgmt_pool_query_dead_ranks_out; +uint64_t ds_mgmt_pool_query_self_heal; +uint32_t ds_mgmt_pool_query_prop_type; int ds_mgmt_pool_query(uuid_t pool_uuid, d_rank_list_t *svc_ranks, d_rank_list_t **enabled_ranks, d_rank_list_t **disabled_ranks, d_rank_list_t **dead_ranks, uint64_t deadline, - daos_pool_info_t *pool_info, uint32_t *pool_layout_ver, + daos_pool_info_t *pool_info, daos_prop_t *prop, uint32_t *pool_layout_ver, uint32_t *upgrade_layout_ver, uint64_t *mem_file_bytes) { /* If function is to return with an error, pool_info and ranks will not be filled. */ @@ -321,6 +323,13 @@ ds_mgmt_pool_query(uuid_t pool_uuid, d_rank_list_t *svc_ranks, d_rank_list_t **e ds_mgmt_pool_query_dead_ranks_out = *dead_ranks; } + ds_mgmt_pool_query_prop_type = 0; + if (prop != NULL) { + ds_mgmt_pool_query_prop_type = prop->dpp_entries[0].dpe_type; + if (ds_mgmt_pool_query_prop_type == DAOS_PROP_PO_SELF_HEAL) + prop->dpp_entries[0].dpe_val = ds_mgmt_pool_query_self_heal; + } + ds_mgmt_pool_query_info_in = *pool_info; *pool_info = ds_mgmt_pool_query_info_out; *mem_file_bytes = ds_mgmt_pool_query_mem_bytes; @@ -338,6 +347,8 @@ mock_ds_mgmt_pool_query_setup(void) ds_mgmt_pool_query_enabled_ranks_out = NULL; ds_mgmt_pool_query_disabled_ranks_out = NULL; ds_mgmt_pool_query_dead_ranks_out = NULL; + ds_mgmt_pool_query_self_heal = 0; + ds_mgmt_pool_query_prop_type = 0; } int ds_mgmt_pool_query_targets_return; diff --git a/src/mgmt/tests/mocks.h b/src/mgmt/tests/mocks.h index c40e4cdab01..4f4ba30b50f 100644 --- a/src/mgmt/tests/mocks.h +++ b/src/mgmt/tests/mocks.h @@ -1,6 +1,6 @@ /* * (C) Copyright 2019-2024 Intel Corporation. - * (C) Copyright 2025 Hewlett Packard Enterprise Development LP + * (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -113,6 +113,8 @@ extern void *ds_mgmt_pool_query_info_ptr; extern d_rank_list_t *ds_mgmt_pool_query_enabled_ranks_out; extern d_rank_list_t *ds_mgmt_pool_query_disabled_ranks_out; extern d_rank_list_t *ds_mgmt_pool_query_dead_ranks_out; +extern uint64_t ds_mgmt_pool_query_self_heal; +extern uint32_t ds_mgmt_pool_query_prop_type; void mock_ds_mgmt_pool_query_setup(void); diff --git a/src/mgmt/tests/srv_drpc_tests.c b/src/mgmt/tests/srv_drpc_tests.c index 38591e39305..9588fd5b3e0 100644 --- a/src/mgmt/tests/srv_drpc_tests.c +++ b/src/mgmt/tests/srv_drpc_tests.c @@ -1557,6 +1557,77 @@ test_drpc_pool_query_rebuild_done_success(void **state) D_FREE(resp.body.data); } +static Mgmt__PoolQueryResp * +unpack_query_resp(Drpc__Response *resp) +{ + Mgmt__PoolQueryResp *pq_resp; + + assert_int_equal(resp->status, DRPC__STATUS__SUCCESS); + pq_resp = mgmt__pool_query_resp__unpack(NULL, resp->body.len, resp->body.data); + assert_non_null(pq_resp); + assert_int_equal(pq_resp->status, 0); + return pq_resp; +} + +static void +test_drpc_pool_query_with_props(void **state) +{ + Drpc__Call call = DRPC__CALL__INIT; + Drpc__Response resp = DRPC__RESPONSE__INIT; + Mgmt__PoolQueryReq req = MGMT__POOL_QUERY_REQ__INIT; + Mgmt__PoolQueryResp *pq_resp; + daos_pool_info_t exp_info = {0}; + + init_test_pool_info(&exp_info); + ds_mgmt_pool_query_info_out = exp_info; + ds_mgmt_pool_query_self_heal = DAOS_SELF_HEAL_AUTO_EXCLUDE; + + req.id = TEST_UUID; + req.query_mask = DEFAULT_QUERY_BITS; + req.properties = alloc_prop_msg_list(1); + req.n_properties = 1; + req.properties[0]->number = DAOS_PROP_PO_SELF_HEAL; + pack_pool_query_req(&call, &req); + + ds_mgmt_drpc_pool_query(&call, &resp); + + assert_int_equal(ds_mgmt_pool_query_prop_type, DAOS_PROP_PO_SELF_HEAL); + pq_resp = unpack_query_resp(&resp); + assert_int_equal(pq_resp->n_properties, 1); + assert_int_equal(pq_resp->properties[0]->number, DAOS_PROP_PO_SELF_HEAL); + assert_int_equal(pq_resp->properties[0]->value_case, MGMT__POOL_PROPERTY__VALUE_NUMVAL); + assert_int_equal(pq_resp->properties[0]->numval, DAOS_SELF_HEAL_AUTO_EXCLUDE); + mgmt__pool_query_resp__free_unpacked(pq_resp, NULL); + + free_prop_msg_list(req.properties, req.n_properties); + D_FREE(call.body.data); + D_FREE(resp.body.data); +} + +static void +test_drpc_pool_query_without_props(void **state) +{ + Drpc__Call call = DRPC__CALL__INIT; + Drpc__Response resp = DRPC__RESPONSE__INIT; + Mgmt__PoolQueryResp *pq_resp; + daos_pool_info_t exp_info = {0}; + + init_test_pool_info(&exp_info); + ds_mgmt_pool_query_info_out = exp_info; + + setup_pool_query_drpc_call(&call, TEST_UUID, 0); + + ds_mgmt_drpc_pool_query(&call, &resp); + + assert_int_equal(ds_mgmt_pool_query_prop_type, 0); + pq_resp = unpack_query_resp(&resp); + assert_int_equal(pq_resp->n_properties, 0); + mgmt__pool_query_resp__free_unpacked(pq_resp, NULL); + + D_FREE(call.body.data); + D_FREE(resp.body.data); +} + static void test_drpc_pool_query_rebuild_busy_success(void **state) { @@ -3622,6 +3693,8 @@ main(void) QUERY_TEST(test_drpc_pool_query_mgmt_svc_fails), QUERY_TEST(test_drpc_pool_query_rebuild_idle_success), QUERY_TEST(test_drpc_pool_query_rebuild_done_success), + QUERY_TEST(test_drpc_pool_query_with_props), + QUERY_TEST(test_drpc_pool_query_without_props), QUERY_TEST(test_drpc_pool_query_rebuild_busy_success), QUERY_TEST(test_drpc_pool_query_rebuild_idle_err), QUERY_TEST(test_drpc_pool_query_rebuild_done_err), diff --git a/src/pool/srv_cli.c b/src/pool/srv_cli.c index 68c48e394cd..937f1ab6adf 100644 --- a/src/pool/srv_cli.c +++ b/src/pool/srv_cli.c @@ -341,6 +341,7 @@ struct pool_query_arg { d_rank_list_t **pqa_disabled_ranks; d_rank_list_t **pqa_dead_ranks; daos_pool_info_t *pqa_info; + daos_prop_t *pqa_prop; uint32_t *pqa_layout_ver; uint32_t *pqa_upgrade_layout_ver; crt_bulk_t pqa_bulk; @@ -361,7 +362,7 @@ pool_query_init(uuid_t pool_uuid, crt_rpc_t *rpc, void *varg) in = crt_req_get(rpc); uuid_copy(in->pqi_op.pi_uuid, pool_uuid); uuid_clear(in->pqi_op.pi_hdl); - query_bits = pool_query_bits(arg->pqa_info, NULL); + query_bits = pool_query_bits(arg->pqa_info, arg->pqa_prop); rc = map_bulk_create(info->dmi_ctx, &arg->pqa_bulk, &arg->pqa_map_buf, arg->pqa_map_size); if (rc != 0) @@ -540,6 +541,8 @@ pool_query_consume(uuid_t pool_uuid, crt_rpc_t *rpc, void *varg) arg->pqa_enabled_ranks, arg->pqa_disabled_ranks, arg->pqa_dead_ranks, arg->pqa_info, pool_uuid, out->pqo_op.po_map_version, out->pqo_op.po_hint.sh_rank, &out->pqo_space, &out->pqo_rebuild_st, arg->pqa_map_buf, arg->pqa_info->pi_bits); + if (rc == 0 && arg->pqa_prop != NULL) + rc = daos_prop_copy(arg->pqa_prop, out->pqo_prop); if (arg->pqa_layout_ver) *arg->pqa_layout_ver = out->pqo_pool_layout_ver; if (arg->pqa_upgrade_layout_ver) @@ -580,6 +583,8 @@ static struct dsc_pool_svc_call_cbs pool_query_cbs = { * \param[out] dead_ranks Optional, storage ranks marked as DEAD by the SWIM * protocol, but were not excluded from the system. * \param[in][out] pool_info Results of the pool query + * \param[in][out] prop Optional, properties to query (entry types set + * by the caller, values filled in on success) * \param[in][out] pool_layout_ver Results of the current pool global version * \param[in][out] upgrade_layout_ver Results of the target latest pool global version * @@ -593,7 +598,7 @@ static struct dsc_pool_svc_call_cbs pool_query_cbs = { int dsc_pool_svc_query(uuid_t pool_uuid, d_rank_list_t *ps_ranks, uint64_t deadline, d_rank_list_t **enabled_ranks, d_rank_list_t **disabled_ranks, - d_rank_list_t **dead_ranks, daos_pool_info_t *pool_info, + d_rank_list_t **dead_ranks, daos_pool_info_t *pool_info, daos_prop_t *prop, uint32_t *pool_layout_ver, uint32_t *upgrade_layout_ver, uint64_t *mem_file_bytes) { @@ -602,6 +607,7 @@ dsc_pool_svc_query(uuid_t pool_uuid, d_rank_list_t *ps_ranks, uint64_t deadline, .pqa_disabled_ranks = disabled_ranks, .pqa_dead_ranks = dead_ranks, .pqa_info = pool_info, + .pqa_prop = prop, .pqa_layout_ver = pool_layout_ver, .pqa_upgrade_layout_ver = upgrade_layout_ver, .pqa_mem_file_bytes = mem_file_bytes, diff --git a/src/proto/mgmt/pool.proto b/src/proto/mgmt/pool.proto index 548aa985f64..0e88642f48e 100644 --- a/src/proto/mgmt/pool.proto +++ b/src/proto/mgmt/pool.proto @@ -189,6 +189,7 @@ message PoolQueryReq { string id = 2; repeated uint32 svc_ranks = 3; // List of pool service ranks uint64 query_mask = 4; // Bitmask of pool query options + repeated PoolProperty properties = 5; // Pool properties to return with the query } enum StorageMediaType { @@ -260,8 +261,8 @@ message PoolQueryResp { uint64 mem_file_bytes = 21; // per-pool accumulated value of memory file sizes string dead_ranks = 22; // optional set of dead ranks bool md_on_ssd_active = 23; // MD-on-SSD mode flag - string self_heal_policy = 24; // pool property self_heal value - string sys_self_heal_policy = 25; // system property self_heal value + repeated PoolProperty properties = 24; // requested pool properties + string sys_self_heal_policy = 25; // system property self_heal value } message PoolProperty { diff --git a/src/tests/ftest/control/dmg_pool_query_test.yaml b/src/tests/ftest/control/dmg_pool_query_test.yaml index f7fb499b84e..3f11c6ba17d 100644 --- a/src/tests/ftest/control/dmg_pool_query_test.yaml +++ b/src/tests/ftest/control/dmg_pool_query_test.yaml @@ -32,7 +32,7 @@ exp_vals: version: 1 leader: 0 replicas: [0] - query_mask: "disabled_engines,rebuild,self_heal_policy,space" + query_mask: "disabled_engines,rebuild,space" self_heal_policy: "exclude;rebuild" scm: total: 16000008192