DAOS-16721 dtx: handle potential DTX ID reusing trouble #15408

Nasf-Fan · 2024-10-28T06:16:57Z

The patch contains the following improvements:

When VOS level logic returns -DER_TX_RESATRT, the object level RPC handler should set 'RESEND' flag then restart the transaction with newer epoch. Because dtx_abort() logic cannot guarantee all former prepared DTX entries (on all related participants) can be aborted, especially if the former one failed for some network trouble, that may cause restarted transaction hit -DER_TX_ID_REUSED unexpectedly.
Compare the epoch for DTX entries with the same transaction ID for distinguishing potential reused TX ID more accurately.
Add DTX entry into DTX CoS cache if cannot commit it synchronously. Then subsequent batched commit logic can handle it.
If server complains suspected TX ID reusing, then reports -EIO to related application instead of assertion on client.
Control DTX related warning message frequency to avoid log flood.
Collect more information when generate some error/warning message.

Allow-unstable-test: 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-10-28T06:17:23Z

Ticket title is 'Aurora: mdtest assertion on punch with 530 servers, 2048 * 104 clients'
Status is 'In Progress'
Labels: 'pre_acceptance_issues'
Job should run at elevated priority (1)
https://daosio.atlassian.net/browse/DAOS-16721

daosbuild1 · 2024-10-28T06:32:47Z

Test stage Unit Test bdev with memcheck on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/1/display/redirect

daosbuild1 · 2024-10-28T06:32:48Z

Test stage Unit Test bdev on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/1/display/redirect

daosbuild1 · 2024-10-28T06:32:49Z

Test stage Unit Test with memcheck on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/1/display/redirect

liuxuezhao · 2024-10-29T03:13:18Z

src/object/cli_obj.c

+		if (task->dt_result == -DER_TX_ID_REUSED && obj_auxi->retry_cnt != 0) {
+			D_ERROR("Be complained as TX ID reused for unknown reason, "
+				"task %p, opc %u, flags %x, retry_cnt %u\n",
+				task, obj_auxi->opc, obj_auxi->flags, obj_auxi->retry_cnt);


maybe some other bug not found some where?
the "|= ORF_RESEND" change below suppose can fix it?

According to the test log, the obj_auxi->flags has already contained RESEND. It is the server side logic reset such flag by wrong.

Do we actually see this error with this patch?

The patch contains the following improvements: 1. When VOS level logic returns -DER_TX_RESATRT, the object level RPC handler should set 'RESEND' flag then restart the transaction with newer epoch. Because dtx_abort() logic cannot guarantee all former prepared DTX entries (on all related participants) can be aborted, especially if the former one failed for some network trouble, that may cause restarted transaction hit -DER_TX_ID_REUSED unexpectedly. 2. Compare the epoch for DTX entries with the same transaction ID for distinguishing potential reused TX ID more accurately. 3. Add DTX entry into DTX CoS cache if cannot commit it synchronously. Then subsequent batched commit logic can handle it. 4. If server complains suspected TX ID reusing, then reports -EIO to related application instead of assertion on client. 5. Control DTX related warning message frequency to avoid log flood. 6. Collect more information when generate some error/warning message. Allow-unstable-test: true Signed-off-by: Fan Yong <[email protected]>

daosbuild1 · 2024-11-02T05:38:36Z

Test stage Functional Hardware Medium Verbs Provider completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/7/display/redirect

daosbuild1 · 2024-11-02T05:42:42Z

Test stage Functional Hardware Large completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/7/display/redirect

daosbuild1 · 2024-11-02T08:47:18Z

Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15408/7/execution/node/1469/log

daosbuild1 · 2024-11-02T20:09:14Z

Test stage Functional Hardware Large completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/8/display/redirect

daosbuild1 · 2024-11-02T20:09:26Z

Test stage Functional Hardware Medium Verbs Provider completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/8/display/redirect

daosbuild1 · 2024-11-03T07:27:49Z

Test stage Functional Hardware Medium Verbs Provider completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/9/display/redirect

daosbuild1 · 2024-11-03T07:28:12Z

Test stage Functional Hardware Large completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/9/display/redirect

daosbuild1 · 2024-11-04T06:26:04Z

Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/10/display/redirect

daosbuild1 · 2024-11-04T06:26:57Z

Test stage Functional Hardware Medium Verbs Provider completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/10/display/redirect

daosbuild1 · 2024-11-04T11:18:08Z

Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/11/display/redirect

phender · 2024-11-04T21:35:05Z

https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/11/ ran into a problem provisioning the nodes for the Functional Hardware Medium stage so it did not run. I've manually kicked off https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/12/ to run just the Functional Hardware Medium stage.

daosbuild1 · 2024-11-04T23:02:04Z

Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/12/display/redirect

gnailzenh · 2024-11-05T14:09:01Z

src/object/cli_obj.c


 	/*
-	 * Randomly delay 5 - 68 us if it is not the first retry for
+	 * Randomly delay 5 ~ 1028 us if it is not the first retry for
 	 * -DER_INPROGRESS || -DER_UPDATE_AGAIN cases.


I think RPC should use millisecond instead of microsecond?

This is general logic for RPC retry because of non-committed DTX, that usually happens for fetch case. The next resent RPC maybe be sent to related DTX leader on which the DTX maybe committable. If we use millisecond, the delay maybe too large.

gnailzenh · 2024-11-05T14:12:20Z

src/object/cli_obj.c

+		if (task->dt_result == -DER_TX_ID_REUSED && obj_auxi->retry_cnt != 0) {
+			D_ERROR("Be complained as TX ID reused for unknown reason, "
+				"task %p, opc %u, flags %x, retry_cnt %u\n",
+				task, obj_auxi->opc, obj_auxi->flags, obj_auxi->retry_cnt);


Do we actually see this error with this patch?

gnailzenh · 2024-11-05T14:24:16Z

src/object/obj_utils.c

 		 */
-		pos = rand % (ocdc->tgt_nr - ocdc->cur_pos) + ocdc->cur_pos;
+		pos = oid.lo % (ocdc->tgt_nr - ocdc->cur_pos) + ocdc->cur_pos;


I really think we should hash oid.lo instead of using it directly.

We hope that the relay engine helping to forwarding collective punch RPC for different objects can be changed instead of always using the same engine(s), then the load can be relative balanced. So either oid.lo or its hash seems both fine for that.

gnailzenh · 2024-11-05T14:24:39Z

src/object/obj_utils.c

-	/* Randomly choose a XS as the local leader on target engine for load balance. */
-	for (i = 0, pos = (rand != 0 ? rand : d_rand()) % dct->dct_tgt_nr; i < size; i++) {
+	/* Choose a target as the local agent on the engine for load balance. */
+	for (i = 0, pos = oid.lo % dct->dct_tgt_nr; i < size; i++) {


gnailzenh · 2024-11-05T14:30:22Z

src/object/srv_ec_aggregate.c

+		/* Report warning per each 10 seconds to avoid log flood. */
+		if (now - cont->sc_ec_agg_busy_ts > 10) {
+			while ((dsp = d_list_pop_entry(&dth->dth_share_tbd_list,
+						       struct dtx_share_peer, dsp_link)) != NULL) {


I don't think it's necessary to dump all TX ids is necessary, probably just a few of them and the total number should be good enough?

There will be at most 4 entries per 10 seconds. It will not cause too much overhead.

gnailzenh · 2024-11-05T14:32:07Z

src/object/srv_ec_aggregate.c

@@ -2715,8 +2720,32 @@ cont_ec_aggregate_cb(struct ds_cont_child *cont, daos_epoch_range_t *epr,
 	agg_reset_entry(&ec_agg_param->ap_agg_entry, NULL, NULL);

 retry:
+	epoch.oe_value = epr->epr_hi;
+	rc = dtx_begin(cont->sc_hdl, &dti, &epoch, 0, cont->sc_pool->spc_map_version, &oid,


the dth is just for returning tbd_list or for other reasons?

For this case, the dth is just for returning tbd_list. But for other cases, it will have more usage.

Nasf-Fan · 2024-11-05T15:05:20Z

Do we actually see this error with this patch?

Up to now, it is not. Because if we hit this error, -EIO will be returned to the application, then related mdtest will fail.

phender · 2024-11-05T15:49:13Z

Functional HW Medium testing passed in https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15408/14/

tanabarr

I don't have a full understanding of the code area but from what I can understand LGTM

github-actions bot added the priority Ticket has high priority (automatically managed) label Oct 28, 2024

Nasf-Fan marked this pull request as ready for review October 28, 2024 14:56

Nasf-Fan requested review from a team as code owners October 28, 2024 14:56

Nasf-Fan requested review from liuxuezhao and gnailzenh and removed request for a team October 28, 2024 14:56

liuxuezhao previously approved these changes Oct 29, 2024

View reviewed changes

Nasf-Fan dismissed liuxuezhao’s stale review via 577c694 October 31, 2024 16:44

Nasf-Fan force-pushed the Nasf-Fan/DAOS-16721_2 branch from 49f5589 to 577c694 Compare October 31, 2024 16:44

Nasf-Fan force-pushed the Nasf-Fan/DAOS-16721_2 branch from 577c694 to a208677 Compare November 1, 2024 17:13

gnailzenh approved these changes Nov 5, 2024

View reviewed changes

Nasf-Fan requested a review from liuxuezhao November 5, 2024 15:06

tanabarr approved these changes Nov 5, 2024

View reviewed changes

phender merged commit 35ee55e into master Nov 5, 2024
54 of 55 checks passed

phender deleted the Nasf-Fan/DAOS-16721_2 branch November 5, 2024 20:04

mjmac mentioned this pull request Nov 13, 2024

mjmac/DAOS 16787 google 2.6 #15498

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAOS-16721 dtx: handle potential DTX ID reusing trouble #15408

DAOS-16721 dtx: handle potential DTX ID reusing trouble #15408

Nasf-Fan commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 28, 2024

daosbuild1 commented Oct 28, 2024

daosbuild1 commented Oct 28, 2024

daosbuild1 commented Oct 28, 2024

liuxuezhao Oct 29, 2024

Nasf-Fan Oct 29, 2024

gnailzenh Nov 5, 2024

daosbuild1 commented Nov 2, 2024

daosbuild1 commented Nov 2, 2024

daosbuild1 commented Nov 2, 2024

daosbuild1 commented Nov 2, 2024

daosbuild1 commented Nov 2, 2024

daosbuild1 commented Nov 3, 2024

daosbuild1 commented Nov 3, 2024

daosbuild1 commented Nov 4, 2024

daosbuild1 commented Nov 4, 2024

daosbuild1 commented Nov 4, 2024

phender commented Nov 4, 2024

daosbuild1 commented Nov 4, 2024

gnailzenh Nov 5, 2024

Nasf-Fan Nov 5, 2024

gnailzenh Nov 5, 2024

gnailzenh Nov 5, 2024

Nasf-Fan Nov 5, 2024

gnailzenh Nov 5, 2024

gnailzenh Nov 5, 2024

Nasf-Fan Nov 5, 2024

gnailzenh Nov 5, 2024

Nasf-Fan Nov 5, 2024

Nasf-Fan commented Nov 5, 2024 •

edited

Loading

phender commented Nov 5, 2024

tanabarr left a comment

DAOS-16721 dtx: handle potential DTX ID reusing trouble #15408

DAOS-16721 dtx: handle potential DTX ID reusing trouble #15408

Conversation

Nasf-Fan commented Oct 28, 2024 • edited Loading

Before requesting gatekeeper:

Gatekeeper:

github-actions bot 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

daosbuild1 commented Nov 2, 2024

daosbuild1 commented Nov 2, 2024

daosbuild1 commented Nov 2, 2024

daosbuild1 commented Nov 2, 2024

daosbuild1 commented Nov 2, 2024

daosbuild1 commented Nov 3, 2024

daosbuild1 commented Nov 3, 2024

daosbuild1 commented Nov 4, 2024

daosbuild1 commented Nov 4, 2024

daosbuild1 commented Nov 4, 2024

phender commented Nov 4, 2024

daosbuild1 commented Nov 4, 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

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

Nasf-Fan commented Nov 5, 2024 • edited Loading

phender commented Nov 5, 2024

tanabarr left a comment

Choose a reason for hiding this comment

Nasf-Fan commented Oct 28, 2024 •

edited

Loading

Nasf-Fan commented Nov 5, 2024 •

edited

Loading