DAOS-16386 utils: Add DDB Feature and RM_POOL Support #15062

wangshilong · 2024-09-04T09:44:48Z

This PR enhances the DDB functionality for CR purposes with the following updates:

Pool Behavior Control:

Administrators can now control certain vos pool behaviors, such as skipping vos pool loading or setting a vos pool to immutable mode.

Manual Pool Shard Removal:

A new command ddb rm_pool <vos_pool> has been introduced, allowing administrators to manually remove pool shards.

SPDK Environment Initialization Bug Fix:

Fixed an issue where spdk_env_init() would fail during reinitialization.

These updates aim to improve system flexibility and stability, providing administrators with more robust management capabilities.

Required-githooks: true
Signed-off-by: Wang Shilong shilong.wang@intel.com

Required-githooks: true

Before requesting gatekeeper:

Two review approvals and any prior change requests have been resolved.
Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
Commit messages follows the guidelines outlined here.
Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

github-actions · 2024-09-04T09:45:09Z

Ticket title is 'Enhance DDB for CR purpose'
Status is 'In Review'
https://daosio.atlassian.net/browse/DAOS-16386

daosbuild1 · 2024-09-04T10:30:04Z

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

daosbuild1 · 2024-09-26T00:15:39Z

Test stage Functional Hardware Medium completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-15062/3/testReport/

This PR enhances the DDB functionality for CR purposes with the following updates: 1. Pool Behavior Control: Administrators can now control certain vos pool behaviors, such as skipping vos pool loading or setting a vos pool to immutable mode. 2. Manual Pool Shard Removal: A new command ddb rm_pool <vos_pool> has been introduced, allowing administrators to manually remove pool shards. 3. SPDK Environment Initialization Bug Fix: Fixed an issue where spdk_env_init() would fail during reinitialization. These updates aim to improve system flexibility and stability, providing administrators with more robust management capabilities. Required-githooks: true Signed-off-by: Wang Shilong <shilong.wang@intel.com> Required-githooks: true

tanabarr

LGTM, is there a reason we couldn't use go-flags for the go cli front-end and instead had to pull in "grumble" package?

wangshilong · 2024-10-08T07:27:17Z

LGTM, is there a reason we couldn't use go-flags for the go cli front-end and instead had to pull in "grumble" package?

I tried to follow @ryon-jensen , maybe he could help answer this.

…S-16386 Required-githooks: true

kjacque

Go changes look reasonable to me, but I'm not sure what everything in the ddb C code is doing. I think we need to get someone more experienced with ddb and/or vos in general to review the other portion of the changes.

src/control/cmd/ddb/main.go

kjacque · 2024-10-09T23:47:01Z

src/utils/ddb/ddb_main.c

+out:
+	return rc;
+}
+
 int
 ddb_main(struct ddb_io_ft *io_ft, int argc, char *argv[])


A little confused because I can't figure out what ddb_main is used for. It seems to think it is the main function of ddb, but the Go stuff is the entry point now. No references to ddb_main outside of this directory. Should it (and the superfluous main function in ddb_entry.c) be removed?

Yes, i am also a bit confused about not sure about this, i think maybe we could remove it, but i'd like to keep it is for now, and remove it in the separate discussion if you are fine with this.

OK. I won't block either way. It would be great to prune unused code whenever we can.

Nasf-Fan · 2024-10-14T15:20:46Z

src/include/daos_srv/vos.h

@@ -20,6 +20,56 @@
 #include <daos_srv/dtx_srv.h>
 #include <daos_srv/vos_types.h>

+#define VOS_POOL_COMPAT_FLAG_IMMUTABLE (1ULL << 0)
+#define VOS_POOL_COMPAT_FLAG_SKIP_LOAD (1ULL << 1)


The name "skip_load" seems some confused, skip load for what? According to the patch, seems the pool will not be started if the flag is set. So maybe called "skip_start" or just "skip_pool" or some more clear name?

I mean skip load, let's change it to start start.

Nasf-Fan · 2024-10-14T15:21:01Z

src/include/daos_srv/vos.h

+#define VOS_POOL_COMPAT_FLAG_IMMUTABLE (1ULL << 0)
+#define VOS_POOL_COMPAT_FLAG_SKIP_LOAD (1ULL << 1)
+#define VOS_POOL_COMPAT_FLAG_SKIP_REBUILD    (1ULL << 2)
+#define VOS_POOL_COMPAT_FLAG_SKIP_DTX_RESYNC (1ULL << 3)


Rebuild depends on DTX resync, if DTX resync is skipped, then rebuild also needs to be skipped.

So we only need FLAG to DTX resync then, it will disable rebuild.

src/utils/ddb/ddb_commands.c

Nasf-Fan · 2024-10-14T15:22:23Z

src/utils/ddb/ddb_commands.c

+ddb_run_rm_pool(struct ddb_ctx *ctx, struct rm_pool_options *opt)
+{
+	if (ddb_pool_is_open(ctx)) {
+		ddb_error(ctx, "Must close pool before can open another\n");


What means? the pool to be destroyed is opened or some other pool is opened?

src/utils/ddb/ddb_main.c

Nasf-Fan · 2024-10-14T15:25:24Z

src/utils/ddb/ddb_vos.h

@@ -48,8 +48,18 @@ struct ddb_array {
 };

 /* Open and close a pool for a ddb_ctx */
-int dv_pool_open(char *path, daos_handle_t *poh);
+int
+    dv_pool_open(const char *path, daos_handle_t *poh, uint32_t flags);


Please adjust related format.

It is a bit annoying, this is auto-format by clang format, let me fix this.

git hooks required this..i leave it for now.

Nasf-Fan · 2024-10-14T15:43:01Z

src/utils/ddb/ddb_vos.c

@@ -50,6 +49,35 @@ dv_pool_open(char *path, daos_handle_t *poh)
 	return rc;
 }

+int
+dv_pool_destroy(const char *path)


Would you please to add some test cases for the pool destroy via ddb? On the other hand, since the DDB tool can be used for recovery from disaster, if SMD information does not match pool case or say SMD is corrupted, can we still operate on the pool by force?

Nasf-Fan · 2024-10-14T15:51:25Z

src/utils/ddb/ddb_commands.c

+	    opt->clear_incompat_flags)
+		ctx->dc_write_mode = true;
+	else
+		ctx->dc_write_mode = false;


Can the feature option be used together with open (with -w) option?

…S-16386 Required-githooks: true

Required-githooks: true Signed-off-by: Wang Shilong <shilong.wang@intel.com>

daosbuild1 · 2024-10-28T02:47:58Z

Test stage Build on Leap 15.5 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15062/6/execution/node/360/log

daosbuild1 · 2024-10-28T02:48:13Z

Test stage Build on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15062/6/execution/node/387/log

daosbuild1 · 2024-10-28T02:49:25Z

Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15062/6/execution/node/273/log

daosbuild1 · 2024-10-28T02:49:37Z

Test stage Build RPM on EL 9 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15062/6/execution/node/363/log

daosbuild1 · 2024-10-28T02:51:06Z

Test stage Build RPM on Leap 15.5 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15062/6/execution/node/326/log

daosbuild1 · 2024-10-28T02:54:37Z

Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15062/6/execution/node/323/log

Required-githooks: true Signed-off-by: Wang Shilong <shilong.wang@intel.com>

Nasf-Fan · 2024-10-29T15:40:26Z

src/utils/ddb/ddb_commands.c

+	if (ddb_pool_is_open(ctx)) {
+		if (!ctx->dc_write_mode && !opt->show_features)
+			return -DER_INVAL;
+		goto skip;


What will happen if the pool is opened without -w option, but some {set,clear}_{in}compat_flags is valid? What is the expected behavior?

it will return -DER_INVAL? maybe should return NOPERM.

Nasf-Fan · 2024-10-29T15:40:48Z

src/utils/ddb/ddb_commands.c

+{
+	if (ddb_pool_is_open(ctx)) {
+		ddb_error(ctx, "Must close pool before can open another\n");
+		return -DER_EXIST;


You want to express that "the pool cannot be destroyed because it is opened" ? if yes, then -DER_BUSY may be better. Or I mis-understand anything?

-DER_BUSY is better.

Nasf-Fan · 2024-10-29T15:41:42Z

src/utils/ddb/ddb_main.c

+	if (str_has_value(pa->pa_r_cmd_run)) {
+		rc = ddb_parse_cmd_str(ctx, pa->pa_r_cmd_run, open);
+		if (rc)
+			return rc;


*open = true for else?

It will be assigned inside ddb_parse_cmd_str().

Required-githooks: true Signed-off-by: Wang Shilong <shilong.wang@intel.com>

…S-16386

daosbuild1 · 2024-10-30T08:02:45Z

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

daosbuild1 · 2024-10-30T08:27:04Z

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

tanabarr · 2024-10-30T12:15:52Z

src/control/cmd/ddb/command_completers.go

+
+func featureCompleter(prefix string, args []string) []string {
+	suggestions := []string{"-h", "-e", "--enable", "-d", "--disable", "-s", "--show"}
+	suggestions = append(suggestions, listDirVos(defMntPrefix)...)


L38-L53 is identical to L57-L72 and should be extracted as a helper function

tanabarr · 2024-10-30T12:20:43Z

src/control/cmd/ddb/commands_wrapper.go

@@ -223,3 +223,36 @@ func ddbDtxActAbort(ctx *DdbContext, path string, dtx_id string) error {
 	/* Run the c code command */
 	return daosError(C.ddb_run_dtx_act_abort(&ctx.ctx, &options))
 }
+
+func ddbFeature(ctx *DdbContext, path string, enable string, disable string, show bool) error {


nit: don't need to specify type for sequential parameters that share type, e.g. can do path, enable, disable string

src/utils/ddb/ddb_commands.c

Required-githooks: true Signed-off-by: Wang Shilong <shilong.wang@intel.com>

kjacque

Go changes LGTM.

This PR enhances the DDB functionality for CR purposes with the following updates: 1. Pool Behavior Control: Administrators can now control certain vos pool behaviors, such as skipping vos pool loading or setting a vos pool to immutable mode. 2. Manual Pool Shard Removal: A new command ddb rm_pool <vos_pool> has been introduced, allowing administrators to manually remove pool shards. 3. SPDK Environment Initialization Bug Fix: Fixed an issue where spdk_env_init() would fail during reinitialization. These updates aim to improve system flexibility and stability, providing administrators with more robust management capabilities. Required-githooks: true Signed-off-by: Wang Shilong <shilong.wang@intel.com>

This PR enhances the DDB functionality for CR purposes with the following updates: 1. Pool Behavior Control: Administrators can now control certain vos pool behaviors, such as skipping vos pool loading or setting a vos pool to immutable mode. 2. Manual Pool Shard Removal: A new command ddb rm_pool <vos_pool> has been introduced, allowing administrators to manually remove pool shards. 3. SPDK Environment Initialization Bug Fix: Fixed an issue where spdk_env_init() would fail during reinitialization. These updates aim to improve system flexibility and stability, providing administrators with more robust management capabilities. Signed-off-by: Wang Shilong <shilong.wang@intel.com>

Nasf-Fan force-pushed the Nasf-Fan/DAOS-16329_1 branch from e1b57a1 to 7011933 Compare September 5, 2024 01:51

Base automatically changed from Nasf-Fan/DAOS-16329_1 to master September 25, 2024 12:21

wangshilong force-pushed the shilongw/DAOS-16386 branch from cc4c66f to d845fda Compare September 29, 2024 06:33

wangshilong marked this pull request as ready for review September 29, 2024 06:33

wangshilong requested review from a team as code owners September 29, 2024 06:33

wangshilong requested review from Nasf-Fan and NiuYawei September 29, 2024 06:33

tanabarr previously approved these changes Sep 30, 2024

View reviewed changes

Merge branch 'master' of github.com:daos-stack/daos into shilongw/DAO…

f7f47d1

…S-16386 Required-githooks: true

kjacque reviewed Oct 9, 2024

View reviewed changes

Nasf-Fan reviewed Oct 14, 2024

View reviewed changes

Wang Shilong added 2 commits October 24, 2024 21:44

Merge branch 'master' of github.com:daos-stack/daos into shilongw/DAO…

d83a0dd

…S-16386 Required-githooks: true

address comments

64ace8b

Required-githooks: true Signed-off-by: Wang Shilong <shilong.wang@intel.com>

wangshilong dismissed tanabarr’s stale review via 64ace8b October 28, 2024 02:42

wangshilong requested review from tanabarr, Nasf-Fan and kjacque October 28, 2024 02:48

Fix build failure

78ba4c8

Required-githooks: true Signed-off-by: Wang Shilong <shilong.wang@intel.com>

Nasf-Fan reviewed Oct 29, 2024

View reviewed changes

Wang Shilong added 2 commits October 30, 2024 02:58

address comments

cf9b23e

Required-githooks: true Signed-off-by: Wang Shilong <shilong.wang@intel.com>

Merge branch 'master' of github.com:daos-stack/daos into shilongw/DAO…

a466518

…S-16386

tanabarr previously approved these changes Oct 30, 2024

View reviewed changes

Nasf-Fan reviewed Oct 31, 2024

View reviewed changes

src/utils/ddb/ddb_commands.c Show resolved Hide resolved

Nasf-Fan self-requested a review October 31, 2024 02:47

address comments

777cd0a

Required-githooks: true Signed-off-by: Wang Shilong <shilong.wang@intel.com>

wangshilong dismissed tanabarr’s stale review via 777cd0a October 31, 2024 06:04

wangshilong requested a review from tanabarr October 31, 2024 06:07

kjacque approved these changes Oct 31, 2024

View reviewed changes

Nasf-Fan approved these changes Nov 1, 2024

View reviewed changes

wangshilong requested a review from a team November 1, 2024 12:05

tanabarr approved these changes Nov 1, 2024

View reviewed changes

NiuYawei merged commit 9aae306 into master Nov 8, 2024
54 of 55 checks passed

NiuYawei deleted the shilongw/DAOS-16386 branch November 8, 2024 02:29

mjmac mentioned this pull request Nov 13, 2024

mjmac/DAOS 16787 google 2.6 #15498

Closed

mjmac mentioned this pull request Jan 13, 2025

Merge release/2.6 into google/2.6 #15725

Merged

mjmac mentioned this pull request Mar 18, 2025

dev/mjmac/b403616653 #16115

Closed

DAOS-16386 utils: Add DDB Feature and RM_POOL Support #15062

DAOS-16386 utils: Add DDB Feature and RM_POOL Support #15062

Conversation

wangshilong commented Sep 4, 2024

Before requesting gatekeeper:

Gatekeeper:

github-actions bot commented Sep 4, 2024 • edited Loading

daosbuild1 commented Sep 4, 2024

daosbuild1 commented Sep 26, 2024

tanabarr left a comment

Choose a reason for hiding this comment

wangshilong commented Oct 8, 2024

kjacque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daosbuild1 commented Oct 28, 2024

daosbuild1 commented Oct 28, 2024

daosbuild1 commented Oct 28, 2024

daosbuild1 commented Oct 28, 2024

daosbuild1 commented Oct 28, 2024

daosbuild1 commented Oct 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daosbuild1 commented Oct 30, 2024

daosbuild1 commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kjacque left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 4, 2024 •

edited

Loading