DAOS-16039 object: fix EC aggregation wrong peer address #14593

liuxuezhao · 2024-06-17T14:19:19Z

Fix EC aggregation wrong peer address when multiple dkeys on same EC agg leader shard.
Change existing test cases to cover that case.

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-06-17T14:19:37Z

Ticket title is 'wrong peer address used for EC aggregation when dkey switched'
Status is 'In Review'
https://daosio.atlassian.net/browse/DAOS-16039

NiuYawei · 2024-06-18T01:03:54Z

src/object/srv_ec_aggregate.c

@@ -2202,11 +2213,12 @@ agg_dkey(daos_handle_t ih, vos_iter_entry_t *entry,
 			DP_UOID(agg_entry->ae_oid), DP_KEY(&agg_entry->ae_dkey),
 			agg_entry->ae_is_leader ? "yes" : "no");
 		agg_reset_dkey_entry(&agg_param->ap_agg_entry, entry);
+		rc = agg_get_obj_handle(agg_entry, true);


Minor: looks moving the peer location setting code in agg_get_obj_handle() to a separate function and call it here would be more clear.

agreed, how about make another cleanup patch, this PR only fix functional logic and make it fix the problem as fast as possible.

NiuYawei · 2024-06-18T01:09:46Z

src/tests/suite/daos_obj_ec.c

+			      DAOS_OBJ_EC_AGG_LEADER_DIFF | DAOS_FAIL_ALWAYS, 0, NULL);
+
+	print_message("wait for 30 seconds for EC aggregation.\n");
+	sleep(30);


Looks old tests used FORCE_EC_AGG fault injection to reduce test time, any reason we replace it with sleep(30)?

old test use FORCE_EC_AGG also with sleep(30), FORCE_EC_AGG flag seems not really useful and will cause EC agg fail with DER_INPROGRESS
And this test case need another fail loc on server side DAOS_OBJ_EC_AGG_LEADER_DIFF, seems cannot set two fail loc at the same time, so I did not use FORCE_EC_AGG

I see, I was thinking the FORCE_EC_AGG is for reducing test time. As for setting two fail locs, we could change aggregation code to force aggregation for you new fail loc (similar to FORCE_EC_AGG).

the ci test report an failure, should be just the ec agg not finished yet.
I'll change as what you suggested force agg for the new fail loc

NiuYawei · 2024-06-18T01:25:40Z

src/object/srv_ec_aggregate.c

@@ -2202,11 +2213,12 @@ agg_dkey(daos_handle_t ih, vos_iter_entry_t *entry,
 			DP_UOID(agg_entry->ae_oid), DP_KEY(&agg_entry->ae_dkey),
 			agg_entry->ae_is_leader ? "yes" : "no");
 		agg_reset_dkey_entry(&agg_param->ap_agg_entry, entry);
+		rc = agg_get_obj_handle(agg_entry, true);


NiuYawei · 2024-06-18T01:28:56Z

src/tests/suite/daos_obj_ec.c

+			      DAOS_OBJ_EC_AGG_LEADER_DIFF | DAOS_FAIL_ALWAYS, 0, NULL);
+
+	print_message("wait for 30 seconds for EC aggregation.\n");
+	sleep(30);


I see, I was thinking the FORCE_EC_AGG is for reducing test time. As for setting two fail locs, we could change aggregation code to force aggregation for you new fail loc (similar to FORCE_EC_AGG).

daosbuild1 · 2024-06-20T13:28:45Z

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

daosbuild1 · 2024-06-21T10:08:14Z

Test stage Functional Hardware Medium UCX Provider completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-14593/3/execution/node/1620/log

daosbuild1 · 2024-06-21T10:35:34Z

Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-14593/3/execution/node/1424/log

daosbuild1 · 2024-06-21T13:08:09Z

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

daosbuild1 · 2024-06-21T14:51:08Z

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-14593/4/testReport/

daosbuild1 · 2024-06-21T20:32:01Z

Test stage Functional Hardware Medium UCX Provider completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-14593/5/execution/node/1578/log

jolivier23 · 2024-06-21T21:20:29Z

src/tests/suite/daos_obj_ec.c

-
-	ec_verify_parity_data(&req, "d_key", "a_key", (daos_size_t)0,
-			      full_update_size, verify_data, DAOS_TX_NONE, true);
+	trigger_and_wait_ec_aggreation_2dkeys(arg, &oid, 1, "d_key1", "d_key2", "a_key", 0,


So to be clear, because this was discussed in chat, there is no way to trigger this with one failure and 1 parity target? Multiple dkeys alone is not a sufficient condition?

multiple dkey is not a sufficient cond, need multiple dkeys share a parity shard and with different parity shard, select the shared shard as agg leader.
"no way to trigger this with one failure" possibly trigger with one failure, but with less possibility.

Fix EC aggregation wrong peer address when multiple dkeys on same EC agg leader shard. Change existing test cases to cover that case. Required-githooks: true Signed-off-by: Xuezhao Liu <xuezhao.liu@intel.com>

Required-githooks: true Signed-off-by: Xuezhao Liu <xuezhao.liu@intel.com>

daosbuild1 · 2024-06-24T05:31:16Z

Test stage Functional on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-14593/6/execution/node/1183/log

daosbuild1 · 2024-06-25T09:44:32Z

Test stage Functional Hardware Medium UCX Provider completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-14593/8/execution/node/1444/log

liuxuezhao · 2024-06-26T01:13:11Z

several failed case is cart_ctl failure due to https://daosio.atlassian.net/browse/DAOS-16008

DAOS-16039 object: fix EC aggregation wrong peer address (#14593) DAOS-16009 rebuild: fix O_TRUNC file size related handling DAOS-15056 rebuild: add rpt to the rgt list properly (#13862) DAOS-15517 rebuild: refine lock handling for rpt list (#14064) DAOS-13812 container: fix destroy vs lookup (#12757) DAOS-15627 dtx: redunce stack usage for DTX resync to avoid overflow (#14189) DAOS-14845 rebuild: do not wait for EC agg for reclaim (#13610) Signed-off-by: Xuezhao Liu <xuezhao.liu@intel.com> Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@intel.com> Signed-off-by: Jeff Olivier <jeffolivier@google.com> Signed-off-by: Wang, Di <wddi218@gmail.com> Signed-off-by: Di Wang <di.wang@intel.com> Signed-off-by: Wang Shilong <shilong.wang@intel.com> Signed-off-by: Fan Yong <fan.yong@intel.com>

…14593) * DAOS-16039 object: fix EC aggregation wrong peer address Fix EC aggregation wrong peer address when multiple dkeys on same EC agg leader shard. Change existing test cases to cover that case. Signed-off-by: Xuezhao Liu <xuezhao.liu@intel.com>

liuxuezhao requested review from a team as code owners June 17, 2024 14:19

liuxuezhao requested review from wangshilong, NiuYawei, gnailzenh and kccain June 17, 2024 14:20

NiuYawei reviewed Jun 18, 2024

View reviewed changes

NiuYawei previously approved these changes Jun 18, 2024

View reviewed changes

wangshilong previously approved these changes Jun 18, 2024

View reviewed changes

liuxuezhao dismissed stale reviews from wangshilong and NiuYawei via 202a57c June 20, 2024 14:15

liuxuezhao force-pushed the lxz/ec_agg_peer_fix branch from 0673518 to 202a57c Compare June 20, 2024 14:15

liuxuezhao requested review from a team as code owners June 20, 2024 14:15

liuxuezhao force-pushed the lxz/ec_agg_peer_fix branch from 202a57c to 4e1d82c Compare June 21, 2024 14:07

jolivier23 reviewed Jun 21, 2024

View reviewed changes

liuxuezhao added 2 commits June 24, 2024 02:37

DAOS-16039 object: fix EC aggregation wrong peer address

42c02f0

Fix EC aggregation wrong peer address when multiple dkeys on same EC agg leader shard. Change existing test cases to cover that case. Required-githooks: true Signed-off-by: Xuezhao Liu <xuezhao.liu@intel.com>

DAOS-16039 object: fix test issue

ab3b6cb

Required-githooks: true Signed-off-by: Xuezhao Liu <xuezhao.liu@intel.com>

liuxuezhao force-pushed the lxz/ec_agg_peer_fix branch from 4e1d82c to ab3b6cb Compare June 24, 2024 02:37

liuxuezhao requested a review from NiuYawei June 26, 2024 01:13

liuxuezhao requested review from wangshilong and jolivier23 June 26, 2024 01:13

wangshilong approved these changes Jun 26, 2024

View reviewed changes

NiuYawei approved these changes Jun 27, 2024

View reviewed changes

liuxuezhao requested a review from a team June 27, 2024 08:44

liuxuezhao added the forced-landing The PR has known failures or has intentionally reduced testing, but should still be landed. label Jun 27, 2024

gnailzenh approved these changes Jun 28, 2024

View reviewed changes

gnailzenh merged commit e03fe47 into master Jun 28, 2024
50 of 53 checks passed

gnailzenh deleted the lxz/ec_agg_peer_fix branch June 28, 2024 07:26

jolivier23 mentioned this pull request Jul 2, 2024

b/349170185 Backport fixes #14680

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAOS-16039 object: fix EC aggregation wrong peer address #14593

DAOS-16039 object: fix EC aggregation wrong peer address #14593

liuxuezhao commented Jun 17, 2024

github-actions bot commented Jun 17, 2024 •

edited

Loading

NiuYawei Jun 18, 2024

liuxuezhao Jun 18, 2024

NiuYawei Jun 18, 2024

NiuYawei Jun 18, 2024

liuxuezhao Jun 18, 2024 •

edited

Loading

NiuYawei Jun 18, 2024

liuxuezhao Jun 20, 2024

NiuYawei Jun 18, 2024

NiuYawei Jun 18, 2024

daosbuild1 commented Jun 20, 2024

daosbuild1 commented Jun 21, 2024

daosbuild1 commented Jun 21, 2024

daosbuild1 commented Jun 21, 2024

daosbuild1 commented Jun 21, 2024

daosbuild1 commented Jun 21, 2024

jolivier23 Jun 21, 2024

liuxuezhao Jun 24, 2024

daosbuild1 commented Jun 24, 2024

daosbuild1 commented Jun 25, 2024

liuxuezhao commented Jun 26, 2024

DAOS-16039 object: fix EC aggregation wrong peer address #14593

DAOS-16039 object: fix EC aggregation wrong peer address #14593

Conversation

liuxuezhao commented Jun 17, 2024

Before requesting gatekeeper:

Gatekeeper:

github-actions bot commented Jun 17, 2024 • edited Loading

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

liuxuezhao Jun 18, 2024 • edited Loading

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 Jun 20, 2024

daosbuild1 commented Jun 21, 2024

daosbuild1 commented Jun 21, 2024

daosbuild1 commented Jun 21, 2024

daosbuild1 commented Jun 21, 2024

daosbuild1 commented Jun 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daosbuild1 commented Jun 24, 2024

daosbuild1 commented Jun 25, 2024

liuxuezhao commented Jun 26, 2024

github-actions bot commented Jun 17, 2024 •

edited

Loading

liuxuezhao Jun 18, 2024 •

edited

Loading