fix: wait legacy projection gap retries in milliseconds, not microseconds (fixes #701) - #704
Merged
Merged
Conversation
Member
Author
|
cc: @lifinsky |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this change proposed?
Why
Warning
Applications that worked around this bug by configuring microseconds (e.g.
500000) must switch back to milliseconds (500). Otherwise each retry now waits 1000× longer.Legacy
#[Projection]gap detection now waits the configured retry delays in milliseconds.Before this change, Ecotone passed the delays unchanged to Prooph, which hands them to
usleep()in microseconds. The default[0, 5, 50, 500, 800]waited about 1.4 ms instead of about 1.4 s. Events from slower concurrent transactions were therefore often skipped for good, which explains rows missing from read models (#701, discussion #605). Prooph resolved its side in prooph/pdo-event-store#263 by keeping microseconds as its contract and only rescaling its own defaults. Ecotone always passes its own delays and never uses Prooph's defaults, so Ecotone has to convert them itself.Before / after
[0, 5, 50, 500, 800][0, 300]against a real gapnull(Prooph defaults)Out of scope
withOption()accept customGapDetectorimplementations (today onlywithOptions()does).Example
Description of Changes
GapDetection::build()converts retry delays from milliseconds to microseconds once, before handing them to Prooph. The stored configuration stays in milliseconds, so compiled containers are unaffected.GapDetectionInSynchronousProjectionTestruns a synchronous projection over a stream with a real gap and asserts that the retry delay is actually waited. It failed before the fix (~20 ms elapsed).Verification: the full PdoEventSourcing suite passes (263 tests, 1 skipped); PHPStan and php-cs-fixer report nothing.
Pull Request Contribution Terms