Repository navigation
Conversation
|
Ticket title is 'rebuild/widely_striped.py:RbldWidelyStriped.test_rebuild_widely_striped - self-heal-property lookup caused a misleading rebuild timeout' |
329e1c8 to
a231fdb
Compare
|
Test stage Functional on EL 9 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19402/1/execution/node/1361/log |
… engine Adds a new system.ErrReplicaNoEngine type that a MS replica may return to a RPC caller to indicate that the caller should retry with one of its peers. Also perform a bit of tech debt cleanup on the MS replica address helpers in the system package. There were multiple implementations of the same logic for getting the set of peer addresses. Features: RbldWidelyStriped Signed-off-by: Michael MacDonald <mjmac@vectorsolutionsgroup.com>
a231fdb to
fa748d6
Compare
|
Since this is rummaging down in the MS, I'd suggest running against "Features: control" as well as the affected test. |
knard38
left a comment
knard38
left a comment
There was a problem hiding this comment.
Nothing blocking, just minor remarks
| "engine stopped": { | ||
| mgmtSvc: noEngine, | ||
| req: &mgmtpb.PoolQueryReq{ | ||
| Id: mockUUID, | ||
| }, | ||
| expErr: &system.ErrReplicaNoEngine{}, |
There was a problem hiding this comment.
Could be nice to also check that the list of replicas to use is properly built with mocking the raft sysdb at line 2513. From my understanding, the default one only have one replica and thus the test will pass even if the list is dropped.
noEngine := newTestMgmtSvc(t, log)
setupTestEngine(t, noEngine.harness.instances[0].(*EngineInstance), 0, true)
// Three replicas (the local one plus two peers), so that the redirect list is not empty.
noEnginePeers := []string{"192.0.2.2:10001", "192.0.2.3:10001"}
noEngine.sysdb = raft.MockDatabaseWithCfg(t, log, &raft.DatabaseConfig{
SystemName: build.DefaultSystemName,
Replicas: []*net.TCPAddr{
common.LocalhostCtrlAddr(),
{IP: net.ParseIP("192.0.2.2"), Port: build.DefaultControlPort},
{IP: net.ParseIP("192.0.2.3"), Port: build.DefaultControlPort},
},
})| "engine stopped": { | |
| mgmtSvc: noEngine, | |
| req: &mgmtpb.PoolQueryReq{ | |
| Id: mockUUID, | |
| }, | |
| expErr: &system.ErrReplicaNoEngine{}, | |
| "engine stopped": { | |
| mgmtSvc: noEngine, | |
| req: &mgmtpb.PoolQueryReq{ | |
| Id: mockUUID, | |
| }, | |
| expErr: &system.ErrReplicaNoEngine{Replicas: noEnginePeers}, |
| func (err *ErrReplicaNoEngine) Error() string { | ||
| return fmt.Sprintf("no running engine on %s replica (try one of %s)", | ||
| build.ManagementServiceName, strings.Join(err.Replicas, ",")) | ||
| } |
There was a problem hiding this comment.
NIT
| func (err *ErrReplicaNoEngine) Error() string { | |
| return fmt.Sprintf("no running engine on %s replica (try one of %s)", | |
| build.ManagementServiceName, strings.Join(err.Replicas, ",")) | |
| } | |
| func (err *ErrReplicaNoEngine) Error() string { | |
| msg := fmt.Sprintf("no running engine on %s replica", build.ManagementServiceName) | |
| if len(err.Replicas) == 0 { | |
| return msg | |
| } | |
| return fmt.Sprintf("%s (try one of %s)", msg, strings.Join(err.Replicas, ",")) | |
| } |
* Adjust ErrReplicaNoEngine message when there are no replicas to try * Adjust engine stopped unit test to use a mock with three replicas Features: control RbldWidelyStriped Signed-off-by: Michael MacDonald <mjmac@vectorsolutionsgroup.com>
Yep, good point. I had to address some other feedback so I included that in the new commit. |
Adds a new system.ErrReplicaNoEngine type that a MS replica may return to
a RPC caller to indicate that the caller should retry with one of its peers.
Also perform a bit of tech debt cleanup on the MS replica address
helpers in the system package. There were multiple implementations of
the same logic for getting the set of peer addresses.
Features: RbldWidelyStriped
Signed-off-by: Michael MacDonald mjmac@vectorsolutionsgroup.com