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

Rework Transaction/Batching handling #2123

Closed
uweschaefer opened this issue Aug 8, 2022 · 4 comments
Closed

Rework Transaction/Batching handling #2123

uweschaefer opened this issue Aug 8, 2022 · 4 comments

Comments

@uweschaefer
Copy link
Member

There is a problem in transaction handling, that on reconnect, a subscription thread might be changed, so that an incomplete transaction (from a factus projection) can casue an inconsistent perception of a current factStreamPosition.
One way to solve this is to commit incomplete transactions on close of a subscription, so that the factStreamPosition gets back to consistent. see #1817

However, The transaction handling part is unnecessary complex and we feel like it leaves too many options for misconfiguration so that we'd suggest to solve this problem in a different way, that will hopefully make things much easier to implement as well as more intutive to understand:

  • on subscription, define a max latency and a max size of batch (always, not only with transactions)
  • deprecate onNext(Fact f) on FactObserver and replace it with onNext(List factBatch)
    ** note that we need to have a default here to keep the compatibility
  • factBatch will be applied within a transaction/batch, if there is an error - everything will always be rolled back
  • simplify ProjectionLenses accrodingly
  • the ReconnectingSubscription has an always consistent view regadring facts applied and factStreamPositions compared to the projection.

The hope is, that most of this can be changed without the necessity to touch application-level code.

@BernhardBln
Copy link
Contributor

Please check if that was maybe caused by #2316

@uweschaefer
Copy link
Member Author

no it wasn't

@uweschaefer
Copy link
Member Author

Decided to add BatchingFactObserver to prevent possible infinite loop in default methods

uweschaefer added a commit that referenced this issue Feb 8, 2024
uweschaefer added a commit that referenced this issue Feb 8, 2024
uweschaefer added a commit that referenced this issue Feb 8, 2024
uweschaefer added a commit that referenced this issue Feb 8, 2024
otbe added a commit that referenced this issue Feb 8, 2024
otbe added a commit that referenced this issue Feb 8, 2024
otbe added a commit that referenced this issue Feb 8, 2024
uweschaefer added a commit that referenced this issue Feb 14, 2024
bedaka pushed a commit that referenced this issue Feb 15, 2024
bedaka pushed a commit that referenced this issue Feb 15, 2024
…ssue2123-transaction-refactoring' into issue2123-transaction-refactoring
uweschaefer added a commit that referenced this issue Feb 15, 2024
uweschaefer added a commit that referenced this issue Feb 15, 2024
bresciamattia pushed a commit that referenced this issue Feb 27, 2024
bresciamattia pushed a commit that referenced this issue Feb 27, 2024
bresciamattia pushed a commit that referenced this issue Feb 27, 2024
uweschaefer added a commit that referenced this issue Feb 27, 2024
…s to delegate to FactObservers. Some Tests still red/commented - not sure if this approach will prevail
@uweschaefer
Copy link
Member Author

this is done by the major rewrite that is part of 0.8

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

No branches or pull requests

2 participants