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.

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

Do not continue to delegate after finalize initiation #519

Merged
merged 1 commit into from
Jan 21, 2025
Merged

Conversation

peterbroadhurst
Copy link
Contributor

Fixes #518

Inspecting this code, it seems like the intent of finalize() (linked below) is to:

  1. Initiate a background finalization
  2. Fire an event on success/failure of the finalization

However, it did not seem to be setting the finalizePending flag, or exiting the state reconcile loop.

func (tf *transactionFlow) finalize(ctx context.Context) {
if tf.finalizeRevertReason == "" {
log.L(ctx).Debugf("finalize transaction %s", tf.transaction.ID.String())
} else {
log.L(ctx).Errorf("finalize transaction %s: %s", tf.transaction.ID.String(), tf.finalizeRevertReason)
}
//flush that to the txmgr database
// so that the user can see that it is reverted and so that we stop retrying to assemble and endorse it
tf.syncPoints.QueueTransactionFinalize(
ctx,
tf.transaction.Domain,
tf.domainAPI.Address(),
tf.transaction.ID,
tf.finalizeRevertReason,
func(ctx context.Context) {
//we are not on the main event loop thread so can't update in memory state here.
// need to go back into the event loop
log.L(ctx).Infof("Transaction %s finalize committed", tf.transaction.ID.String())
// Remove this transaction from our domain context on success - all changes are flushed to DB at this point
tf.domainContext.ResetTransactions(tf.transaction.ID)
go tf.publisher.PublishTransactionFinalizedEvent(ctx, tf.transaction.ID.String())
},
func(ctx context.Context, rollbackErr error) {
//we are not on the main event loop thread so can't update in memory state here.
// need to go back into the event loop
log.L(ctx).Errorf("Transaction %s finalize rolled back: %s", tf.transaction.ID.String(), rollbackErr)
// Reset the whole domain context on failure
tf.domainContext.Reset()
go tf.publisher.PublishTransactionFinalizeError(ctx, tf.transaction.ID.String(), tf.finalizeRevertReason, rollbackErr)
},
)
}

@peterbroadhurst peterbroadhurst marked this pull request as ready for review January 20, 2025 20:51
@peterbroadhurst peterbroadhurst requested a review from hosie January 20, 2025 20:51
Copy link
Contributor

@hosie hosie left a comment

Choose a reason for hiding this comment

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

LGTM

@dwertent dwertent enabled auto-merge January 21, 2025 18:39
@dwertent dwertent merged commit 7b55a1d into main Jan 21, 2025
6 checks passed
@dwertent dwertent deleted the fix-518 branch January 21, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Duplicate submission in parallel submission E2E test
3 participants