Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DAOS-10138 pool: Improve PS reconfigurations #10121

Merged
merged 7 commits into from
Sep 27, 2022
Merged

DAOS-10138 pool: Improve PS reconfigurations #10121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2a55a6e
DAOS-10138 pool: Improve PS reconfigurations
liw Aug 16, 2022
1b6fb0a
Address Mike's comment
liw Sep 7, 2022
ac2d4da
Suppress NLT issues
liw Sep 7, 2022
7b32850
Merge branch 'master' into liw/ps-reconf
liw Sep 13, 2022
f97e7cb
Merge branch 'master' into liw/ps-reconf
liw Sep 18, 2022
b2246c4
Address Ken's comments
liw Sep 23, 2022
ea3e3fa
Merge branch 'master' into liw/ps-reconf
liw Sep 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/control/system/raft/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -944,11 +944,27 @@ func (db *Database) UpdatePoolService(ps *system.PoolService) error {
db.Lock()
defer db.Unlock()

_, err := db.FindPoolServiceByUUID(ps.PoolUUID)
p, err := db.FindPoolServiceByUUID(ps.PoolUUID)
if err != nil {
return errors.Wrapf(err, "failed to retrieve pool %s", ps.PoolUUID)
}

// This is a workaround before we can handle the following race
// properly.
//
// mgmtSvc.PoolCreate() Database.handlePoolRepsUpdate()
// Write ps: Creating
// Read ps: Creating
// Write ps: Ready
// Write ps: Creating
//
// The pool remains in Creating state after PoolCreate completes,
// leading to DER_AGAINs during PoolDestroy.
if p.State == system.PoolServiceStateReady && ps.State == system.PoolServiceStateCreating {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me, thanks. This is probably not even really a workaround now, but just the correct behavior. If we are in this situation, all of the information in the "update" is probably stale anyhow.

Copy link
Contributor Author

@liw liw Sep 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not 100% satisfied with this change for 1) the update may contain a valid svc rank refresh as in the example (where the svc ranks could differ with those reported by ds_pool_svc_dist_create in theory), and 2) I return nil in this case (because otherwise callers would need some specific error that they could recognize). The control plane deserves a better solution eventually. :)

db.log.Debugf("ignoring invalid pool service update: %+v -> %+v", p, ps)
return nil
}

if err := db.submitPoolUpdate(raftOpUpdatePoolService, ps); err != nil {
return err
}
Expand Down
8 changes: 5 additions & 3 deletions src/engine/drpc_ras.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright 2021 Intel Corporation.
* (C) Copyright 2021-2022 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand Down Expand Up @@ -183,7 +183,7 @@ log_event(Shared__RASEvent *evt)
out:
fclose(stream);
D_INFO("&&& RAS EVENT%s\n", buf);
D_FREE(buf);
free(buf);
}

static int
Expand Down Expand Up @@ -300,7 +300,7 @@ ds_notify_ras_eventf(ras_event_t id, ras_type_t type, ras_sev_t sev, char *hwid,
}

int
ds_notify_pool_svc_update(uuid_t *pool, d_rank_list_t *svcl)
ds_notify_pool_svc_update(uuid_t *pool, d_rank_list_t *svcl, uint64_t version)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[Note] This is for future use; daos_server doesn't use the field yet.

{
Shared__RASEvent evt = SHARED__RASEVENT__INIT;
Shared__RASEvent__PoolSvcEventInfo info = \
Expand All @@ -323,6 +323,8 @@ ds_notify_pool_svc_update(uuid_t *pool, d_rank_list_t *svcl)
return rc;
}

info.version = version;

evt.extended_info_case = SHARED__RASEVENT__EXTENDED_INFO_POOL_SVC_INFO;
evt.pool_svc_info = &info;

Expand Down
6 changes: 3 additions & 3 deletions src/engine/tests/drpc_client_tests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ test_drpc_verify_notify_pool_svc_update(void **state)
svc_ranks = uint32_array_to_rank_list(svc_reps, 4);
assert_non_null(svc_ranks);

assert_rc_equal(ds_notify_pool_svc_update(&pool_uuid, svc_ranks), 0);
assert_rc_equal(ds_notify_pool_svc_update(&pool_uuid, svc_ranks, 1), 0);
verify_notify_pool_svc_update(&pool_uuid, svc_ranks);

d_rank_list_free(svc_ranks);
Expand All @@ -338,7 +338,7 @@ test_drpc_verify_notify_pool_svc_update_noreps(void **state)
assert_int_equal(uuid_parse("11111111-1111-1111-1111-111111111111",
pool_uuid), 0);

assert_rc_equal(ds_notify_pool_svc_update(&pool_uuid, NULL),
assert_rc_equal(ds_notify_pool_svc_update(&pool_uuid, NULL, 1),
-DER_INVAL);
assert_int_equal(sendmsg_call_count, 0);

Expand All @@ -357,7 +357,7 @@ test_drpc_verify_notify_pool_svc_update_nopool(void **state)
svc_ranks = uint32_array_to_rank_list(svc_reps, 4);
assert_non_null(svc_ranks);

assert_rc_equal(ds_notify_pool_svc_update(NULL, svc_ranks),
assert_rc_equal(ds_notify_pool_svc_update(NULL, svc_ranks, 1),
-DER_INVAL);
assert_int_equal(sendmsg_call_count, 0);

Expand Down
3 changes: 2 additions & 1 deletion src/include/daos_srv/ras.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,12 @@ ds_notify_ras_eventf(ras_event_t id, ras_type_t type, ras_sev_t sev, char *hwid,
*
* \param[in] pool UUID of DAOS pool with updated service replicas.
* \param[in] svcl New list of pool service replica ranks.
* \param[in] version Version of \a svcl.
*
* \retval Zero on success, non-zero otherwise.
*/
int
ds_notify_pool_svc_update(uuid_t *pool, d_rank_list_t *svcl);
ds_notify_pool_svc_update(uuid_t *pool, d_rank_list_t *svcl, uint64_t version);

/**
* Notify control plane that swim has detected a dead rank.
Expand Down
10 changes: 6 additions & 4 deletions src/include/daos_srv/rdb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,11 @@ struct rdb_storage;
struct rdb_cbs;

/** Database storage methods */
int rdb_create(const char *path, const uuid_t uuid, size_t size, const d_rank_list_t *replicas,
struct rdb_cbs *cbs, void *arg, struct rdb_storage **storagep);
int rdb_open(const char *path, const uuid_t uuid, struct rdb_cbs *cbs, void *arg,
struct rdb_storage **storagep);
int rdb_create(const char *path, const uuid_t uuid, uint64_t caller_term, size_t size,
const d_rank_list_t *replicas, struct rdb_cbs *cbs, void *arg,
struct rdb_storage **storagep);
int rdb_open(const char *path, const uuid_t uuid, uint64_t caller_term, struct rdb_cbs *cbs,
void *arg, struct rdb_storage **storagep);
void rdb_close(struct rdb_storage *storage);
int rdb_destroy(const char *path, const uuid_t uuid);

Expand Down Expand Up @@ -165,6 +166,7 @@ int rdb_get_leader(struct rdb *db, uint64_t *term, d_rank_t *rank);
int rdb_get_ranks(struct rdb *db, d_rank_list_t **ranksp);
int rdb_add_replicas(struct rdb *db, d_rank_list_t *replicas);
int rdb_remove_replicas(struct rdb *db, d_rank_list_t *replicas);
int rdb_ping(struct rdb *db, uint64_t caller_term);

/**
* Path (opaque)
Expand Down
27 changes: 14 additions & 13 deletions src/include/daos_srv/rsvc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright 2019-2021 Intel Corporation.
* (C) Copyright 2019-2022 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand Down Expand Up @@ -93,6 +93,8 @@ enum ds_rsvc_state {
DS_RSVC_DOWN /**< down */
};

char *ds_rsvc_state_str(enum ds_rsvc_state state);

/** Replicated service */
struct ds_rsvc {
d_list_t s_entry; /* in rsvc_hash */
Expand Down Expand Up @@ -120,27 +122,26 @@ int ds_rsvc_start_nodb(enum ds_rsvc_class_id class, d_iov_t *id,
uuid_t db_uuid);
int ds_rsvc_stop_nodb(enum ds_rsvc_class_id class, d_iov_t *id);

int ds_rsvc_start(enum ds_rsvc_class_id class, d_iov_t *id, uuid_t db_uuid,
int ds_rsvc_start(enum ds_rsvc_class_id class, d_iov_t *id, uuid_t db_uuid, uint64_t caller_term,
bool create, size_t size, d_rank_list_t *replicas, void *arg);
int ds_rsvc_stop(enum ds_rsvc_class_id class, d_iov_t *id, bool destroy);
int ds_rsvc_stop(enum ds_rsvc_class_id class, d_iov_t *id, uint64_t caller_term, bool destroy);
int ds_rsvc_stop_all(enum ds_rsvc_class_id class);
int ds_rsvc_stop_leader(enum ds_rsvc_class_id class, d_iov_t *id,
struct rsvc_hint *hint);
int ds_rsvc_dist_start(enum ds_rsvc_class_id class, d_iov_t *id,
const uuid_t dbid, const d_rank_list_t *ranks,
bool create, bool bootstrap, size_t size);
int ds_rsvc_dist_stop(enum ds_rsvc_class_id class, d_iov_t *id,
const d_rank_list_t *ranks, d_rank_list_t *excluded,
bool destroy);
int ds_rsvc_dist_start(enum ds_rsvc_class_id class, d_iov_t *id, const uuid_t dbid,
const d_rank_list_t *ranks, uint64_t caller_term, bool create,
bool bootstrap, size_t size);
int ds_rsvc_dist_stop(enum ds_rsvc_class_id class, d_iov_t *id, const d_rank_list_t *ranks,
d_rank_list_t *excluded, uint64_t caller_term, bool destroy);
enum ds_rsvc_state ds_rsvc_get_state(struct ds_rsvc *svc);
void ds_rsvc_set_state(struct ds_rsvc *svc, enum ds_rsvc_state state);
int ds_rsvc_add_replicas_s(struct ds_rsvc *svc, d_rank_list_t *ranks,
size_t size);
int ds_rsvc_add_replicas(enum ds_rsvc_class_id class, d_iov_t *id,
d_rank_list_t *ranks, size_t size,
struct rsvc_hint *hint);
int ds_rsvc_remove_replicas_s(struct ds_rsvc *svc, d_rank_list_t *ranks,
bool stop);
int ds_rsvc_remove_replicas(enum ds_rsvc_class_id class, d_iov_t *id,
d_rank_list_t *ranks, bool stop,
int ds_rsvc_remove_replicas_s(struct ds_rsvc *svc, d_rank_list_t *ranks);
int ds_rsvc_remove_replicas(enum ds_rsvc_class_id class, d_iov_t *id, d_rank_list_t *ranks,
struct rsvc_hint *hint);
int ds_rsvc_lookup(enum ds_rsvc_class_id class, d_iov_t *id,
struct ds_rsvc **svc);
Expand Down
2 changes: 1 addition & 1 deletion src/mgmt/srv_pool.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ ds_mgmt_create_pool(uuid_t pool_uuid, const char *group, char *tgt_dev,
rc_cleanup = ds_mgmt_tgt_pool_destroy_ranks(pool_uuid, targets, true);
if (rc_cleanup)
D_ERROR(DF_UUID": failed to clean up failed pool: "DF_RC"\n",
DP_UUID(pool_uuid), DP_RC(rc));
DP_UUID(pool_uuid), DP_RC(rc_cleanup));
}

out:
Expand Down
Loading