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

Implement the TM provider of Local-TM #34

Merged
merged 50 commits into from
Sep 16, 2022
Merged

Implement the TM provider of Local-TM #34

merged 50 commits into from
Sep 16, 2022

Conversation

JadynWong
Copy link
Contributor

@JadynWong JadynWong commented Jul 29, 2022

  • EFCore DbProvider
  • Mongo DbProvider
  • UTs
  • Docs

@JadynWong JadynWong requested a review from gdlcf88 July 29, 2022 13:26
@JadynWong JadynWong marked this pull request as ready for review September 4, 2022 03:17
Copy link
Member

@gdlcf88 gdlcf88 left a comment

Choose a reason for hiding this comment

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

@JadynWong - What do you think about these:

  1. Create a Stepping.TmProvider.LocalTm.HostedServiceProcessor project. Move the HostedService and the distributed lock to it.
  2. Rename the project Stepping.TmProvider.LocalTm to Stepping.TmProvider.LocalTm.Core.
  3. Create a new Stepping.TmProvider.LocalTm project that depends on Stepping.TmProvider.LocalTm.Core and Stepping.TmProvider.LocalTm.HostedServiceProcessor.

That means if someone doesn't want to use the hosted service but (maybe) Hangfire or else, he doesn't need to care about the implementation of ISteppingDistributedLock and installs only the Stepping.TmProvider.LocalTm.Core package.

@gdlcf88
Copy link
Member

gdlcf88 commented Sep 15, 2022

I created a test app project for local TM. It seems that submitting without invoking the prepare API cause error. Please support this case.

@JadynWong
Copy link
Contributor Author

I created a test app project for local TM. It seems that submitting without invoking the prepare API cause error. Please support this case.

Resolved.

@gdlcf88
Copy link
Member

gdlcf88 commented Sep 15, 2022

@JadynWong - I made this change and expected Step2 could retry after 2s, but it takes about 1min:

if (!canExecuteStep2)
{
canExecuteStep2 = true;
Console.WriteLine("Step 2 deliberately failed to execute and waited for the TM to pick it up.");
context.Response.StatusCode = 500;
return Task.CompletedTask;
}

@gdlcf88
Copy link
Member

gdlcf88 commented Sep 15, 2022

@JadynWong - Great implementation! It's all LGTM.

I added a new to-do "Docs" in the top comment. I believe we can merge this PR when it is completed.

@gdlcf88 gdlcf88 merged commit 7c47927 into main Sep 16, 2022
@gdlcf88 gdlcf88 deleted the jadyn/local-tm branch September 16, 2022 16:26
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.

2 participants