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

Introduce OrleansAssetOccpuancyProvider #42

Closed
wants to merge 2 commits into from
Closed

Conversation

RayMMond
Copy link
Member

@RayMMond RayMMond commented Aug 7, 2022

Resolve #34

@RayMMond RayMMond requested review from gdlcf88 and removed request for gdlcf88 August 7, 2022 13:07
@RayMMond RayMMond requested a review from gdlcf88 August 8, 2022 10:05
@RayMMond
Copy link
Member Author

RayMMond commented Aug 8, 2022

@gdlcf88 plz review this 😃

@gdlcf88
Copy link
Member

gdlcf88 commented Aug 10, 2022

@RayMMond - Pure Orleans Grain usage has better performance. I think there are two better ways:

  1. We don't use the repositories(with UOW, AssetOccupancy, and AssetOccupancyCount) but use Orleans's persistence.
  2. We still use the repositories but cache the state in the grain for better performance.

@RayMMond
Copy link
Member Author

@RayMMond - Pure Orleans Grain usage has better performance. I think there are two better ways:

  1. We don't use the repositories(with UOW, AssetOccupancy, and AssetOccupancyCount) but use Orleans's persistence.
  2. We still use the repositories but cache the state in the grain for better performance.
  1. I believe we should use the repositories for consistency. What if a user using default implementation of AssetOccupancyProvider decide to use Orleans as provider?
  2. With my limited knowledge, the "repositories with state cached" way is hard to keep data synchronized. For example, a silo is crashed and state will be loss since Orleans's onDestroyAsync is not a reliable way to save changed data to database.

@gdlcf88 Any ideas?

@gdlcf88
Copy link
Member

gdlcf88 commented Aug 10, 2022

  1. I believe we should use the repositories for consistency. What if a user using default implementation of AssetOccupancyProvider decide to use Orleans as provider?

I agree that non-breaking migration is more friendly for app developers.

  1. With my limited knowledge, the "repositories with state cached" way is hard to keep data synchronized. For example, a silo is crashed and state will be loss since Orleans's onDestroyAsync is not a reliable way to save changed data to database.

You are right. Consistency and high concurrency are difficult to achieve at the same time. See my new design of EShop's InventoryGrain about delay persisting the inventory state, FYI.

@RayMMond RayMMond removed the request for review from gdlcf88 August 11, 2022 01:49
@RayMMond RayMMond marked this pull request as draft September 3, 2022 11:34
@RayMMond
Copy link
Member Author

RayMMond commented Sep 6, 2022

See #44

@RayMMond RayMMond closed this Sep 6, 2022
@RayMMond RayMMond deleted the orleans-provider branch September 17, 2022 06:22
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.

Introduce Orleans as AssetOccpuancyProvider
2 participants