DAOS-18625 utils: interpret key based to object's type for ddb output by Nasf-Fan · Pull Request #17617 · daos-stack/daos

Nasf-Fan · 2026-02-27T08:17:49Z

Current ddb logic may print some integer keys' output as not-readable, for example dkey 10, 60 will be printed as following: DKEY: (/[4]/[2]/[0]) /b2b37b1a-2894-4970-b51c-1a964526bb02/948289433254841843.256.535.2/ {8} DKEY: (/[4]/[2]/[2]) /b2b37b1a-2894-4970-b51c-1a964526bb02/948289433254841843.256.535.2/<{8}

That is very confused. The patch defines new ddb_key_to_printable_buf() interface that will interpret key based to object's type and generate more readable ddb output.

Test-tag: recovery

Steps for the author:

Commit message follows the guidelines.
Appropriate Features or Test-tag pragmas were used.
Appropriate Functional Test Stages were run.
At least two positive code reviews including at least one code owner from each category referenced in the PR.
Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

Gatekeeper requested (daos-gatekeeper added as a reviewer).

github-actions · 2026-02-27T08:18:11Z

Ticket title is 'Confused ddb ls ouput for integer keys'
Status is 'In Progress'
Labels: 'scrubbed_2.8'
https://daosio.atlassian.net/browse/DAOS-18625

daosbuild3 · 2026-02-27T13:17:34Z

Test stage Functional on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17617/6/execution/node/1078/log

daosbuild3 · 2026-02-27T14:19:56Z

Test stage NLT on EL 8.8 completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-17617/8/testReport/

Current ddb logic may print some integer keys' output as not-readable, for example dkey 10, 60 will be printed as following: DKEY: (/[4]/[2]/[0]) /b2b37b1a-2894-4970-b51c-1a964526bb02/948289433254841843.256.535.2/ {8} DKEY: (/[4]/[2]/[2]) /b2b37b1a-2894-4970-b51c-1a964526bb02/948289433254841843.256.535.2/<{8} That is very confused. The patch defines new ddb_key_to_printable_buf() interface that will interpret key based to object's type and generate more readable ddb output. Test-tag: recovery Signed-off-by: Xuezhao Liu <xuezhao.liu@hpe.com> Signed-off-by: Fan Yong <fan.yong@hpe.com>

daosbuild3 · 2026-03-02T08:51:38Z

Test stage Functional Hardware Large MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17617/9/execution/node/1352/log

liuxuezhao · 2026-03-03T06:31:42Z

looks better to backport to 2.6 when convenient, thx

Nasf-Fan · 2026-03-03T13:23:07Z

looks better to backport to 2.6 when convenient, thx

Yes, the patch for release/2.6 is #17618.

knard38

I am probably missing something, but do not see new tests checking print improvements of this PR.

knard38 · 2026-03-04T08:15:29Z

src/utils/ddb/ddb_tree_path.c

-	d_iov_t *key_iov = &key_part->itp_key;
+itp_print_part_key(struct ddb_ctx *ctx, struct indexed_tree_path_part *part)
+{
+	char     buf[DDB_MAX_PRITABLE_KEY];


NIT

Suggested change

char buf[DDB_MAX_PRITABLE_KEY];

char buf[DDB_MAX_PRINTABLE_KEY];

OK, it was typo in original implementation. I will fix it.

knard38 · 2026-03-04T08:23:15Z

src/utils/ddb/ddb_tree_path.c

+
+	len = ddb_key_to_printable_buf(key_iov, part->itp_otype, buf, ARRAY_SIZE(buf));
+	if (len > ARRAY_SIZE(buf)) {
+		len += 2; /* +2 for escape character if needed and null terminator. */


Not sure to fully understand this +2: I am probably missing something.
From my understanding, escape character need 2 printable characters.
Do not see why we could only have one escape character.
If I am true, then +2 should be evaluated according to the number of escape characters.

You are right, +2 is for the escape character.

knard38 · 2026-03-04T08:26:48Z

src/utils/ddb/ddb_tree_path.c

+	    (!ddb_key_is_int(part->itp_otype) && ddb_can_print(key_iov))) {
 		/* +1 to make sure there's room for a null terminator */
-		char key_str[key_part->itp_key.iov_len + 1];
+		char key_str[key_iov->iov_len + 1];


Are we sure that this size will stay reasonable to allocate key_str on the stack ?

Generally, we should dynamically allocate the buffer instead of static on stack. Current patch just reused original implementation. I will refresh it.

Did you plan to keep this stack allocation or change it to heap allocation as indicated in your comments ?
From my side, no real issue to keep on the stack if we are sure that it will always be reasonable.

knard38 · 2026-03-04T08:31:38Z

src/utils/ddb/ddb_tree_path.c

+	uint32_t len;
+
+	len = ddb_key_to_printable_buf(key_iov, part->itp_otype, buf, ARRAY_SIZE(buf));
+	if (len > ARRAY_SIZE(buf)) {


Why the following +2 should not take into account into this test ?

Suggested change

if (len > ARRAY_SIZE(buf)) {

if (len + 2 > ARRAY_SIZE(buf)) {

Because else len = ARRAY_SIZE(buf);

if len == ARRAY_SIZE(but) and if we have some escape characters then buf will not have enough space from the comment at line 838 ?

src/utils/ddb/ddb_tree_path.c

Nasf-Fan · 2026-03-05T04:33:16Z

I am probably missing something, but do not see new tests checking print improvements of this PR.

Added new test case in check src/utils/ddb/tests/ddb_path_tests.c:
assert_key_parsed_printed("{uint64:10}", "{uint64:0xa}", true);

If without the patch, the out will be something like:
DKEY: (/[4]/[2]/[0]) /b2b37b1a-2894-4970-b51c-1a964526bb02/948289433254841843.256.535.2/ {8}

Please note the invisible output ' ' before {8}. After applying the patch, the output will be:
DKEY: (/[4]/[2]/[0]) /b2b37b1a-2894-4970-b51c-1a964526bb02/948289433254841843.256.535.2/{uint64:0xa}

Test-tag: recovery Signed-off-by: Fan Yong <fan.yong@hpe.com>

daosbuild3 · 2026-03-05T10:18:06Z

Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17617/11/execution/node/1311/log

Nasf-Fan · 2026-03-06T03:02:57Z

CR leader switch cased CR test failure, not related with the ddb patch, to be retested.

knard38 · 2026-03-09T09:30:02Z

src/utils/ddb/ddb_tree_path.c

-		if (!itp_key_safe_str(buf, ARRAY_SIZE(buf))) {
+		if (!itp_key_safe_str(ptr, len)) {
 			ddb_print(ctx, "(ISSUE PRINTING KEY)");
+			D_FREE(key_str);


Should free ptr if needed:

if (ptr != buf) D_FREE(ptr);

Right. I will fix it. Thanks!

Test-tag: recovery Signed-off-by: Fan Yong <fan.yong@hpe.com>

knard38 · 2026-03-10T05:36:47Z

src/utils/ddb/ddb_tree_path.c

+	uint32_t len;
+
+	len = ddb_key_to_printable_buf(key_iov, part->itp_otype, buf, ARRAY_SIZE(buf));
+	if (len > ARRAY_SIZE(buf)) {


Should it be ?

Suggested change

if (len > ARRAY_SIZE(buf)) {

if (len >= ARRAY_SIZE(buf)) {

The logic is stale. I will refresh the patch soon.

Test-tag: recovery Signed-off-by: Fan Yong <fan.yong@hpe.com>

knard38 · 2026-03-12T06:32:28Z

src/utils/ddb/ddb_tree_path.c


-	ddb_iov_to_printable_buf(key_iov, buf, ARRAY_SIZE(buf));
-	if (ddb_can_print(key_iov)) {
+	ddb_key_to_printable_buf(key_iov, part->itp_otype, buf, ARRAY_SIZE(buf));


Should we check if the value return is greater or equal than ARRAY_SIZE(buf).
If yes, maybe we should print a truncation warning message from my understanding.

If the buffer is not large enough to hold the key, it will be automatically truncated with '\0' terminated. The macro DDB_MAX_PRINTABLE_KEY is for case of very long key string.

We can add warning in ddb_key_to_printable_buf() for the key that is longer than DDB_MAX_PRINTABLE_KEY. Since such case (key length > 1024) is very rare and RC1 branch-time is coming soon, I prefer to do that in subsequent patch.

How do you think? @knard38

Nasf-Fan force-pushed the Nasf-Fan/DAOS-18625 branch 6 times, most recently from 23588dd to 847df11 Compare February 27, 2026 13:16

Nasf-Fan force-pushed the Nasf-Fan/DAOS-18625 branch from 847df11 to 2fe1617 Compare February 27, 2026 13:25

Nasf-Fan force-pushed the Nasf-Fan/DAOS-18625 branch from 2fe1617 to 7e0a598 Compare March 2, 2026 02:57

Nasf-Fan force-pushed the Nasf-Fan/DAOS-18625 branch from 7e0a598 to 4c0e467 Compare March 2, 2026 08:50

Nasf-Fan mentioned this pull request Mar 2, 2026

DAOS-18487 utils: refine ddb_iov_to_printable_buf #17594

Closed

6 tasks

Nasf-Fan marked this pull request as ready for review March 3, 2026 03:28

Nasf-Fan requested review from grom72, janekmi, knard38 and liuxuezhao and removed request for grom72 and janekmi March 3, 2026 03:29

liuxuezhao previously approved these changes Mar 3, 2026

View reviewed changes

Nasf-Fan requested a review from gnailzenh March 4, 2026 00:28

knard38 reviewed Mar 4, 2026

View reviewed changes

DAOS-18625 utils: fixes for review feedback

fc1c369

Test-tag: recovery Signed-off-by: Fan Yong <fan.yong@hpe.com>

Nasf-Fan dismissed liuxuezhao’s stale review via fc1c369 March 5, 2026 04:45

Nasf-Fan requested review from knard38 and liuxuezhao March 9, 2026 02:48

liuxuezhao previously approved these changes Mar 9, 2026

View reviewed changes

knard38 reviewed Mar 9, 2026

View reviewed changes

DAOS-18625 utils: fix DRAM leak in former commit

5cc80b6

Test-tag: recovery Signed-off-by: Fan Yong <fan.yong@hpe.com>

Nasf-Fan dismissed liuxuezhao’s stale review via 5cc80b6 March 9, 2026 14:30

Nasf-Fan requested review from knard38 and liuxuezhao March 10, 2026 01:47

knard38 reviewed Mar 10, 2026

View reviewed changes

DAOS-18625 utils: boundary check in itp_key_safe_str

6fa51c8

Test-tag: recovery Signed-off-by: Fan Yong <fan.yong@hpe.com>

Nasf-Fan requested a review from knard38 March 12, 2026 02:21

knard38 reviewed Mar 12, 2026

View reviewed changes

	char buf[DDB_MAX_PRITABLE_KEY];
	char buf[DDB_MAX_PRINTABLE_KEY];

	if (len > ARRAY_SIZE(buf)) {
	if (len + 2 > ARRAY_SIZE(buf)) {

Conversation

Nasf-Fan commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Steps for the author:

After all prior steps are complete:

Uh oh!

github-actions bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daosbuild3 commented Feb 27, 2026

Uh oh!

daosbuild3 commented Feb 27, 2026

Uh oh!

daosbuild3 commented Mar 2, 2026

Uh oh!

liuxuezhao commented Mar 3, 2026

Uh oh!

Nasf-Fan commented Mar 3, 2026

Uh oh!

knard38 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knard38 Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Nasf-Fan commented Mar 5, 2026

Uh oh!

daosbuild3 commented Mar 5, 2026

Uh oh!

Nasf-Fan commented Mar 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nasf-Fan Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Nasf-Fan commented Feb 27, 2026 •

edited

Loading

github-actions bot commented Feb 27, 2026 •

edited

Loading

knard38 Mar 12, 2026 •

edited

Loading

Nasf-Fan Mar 11, 2026 •

edited

Loading