Inject updated TransitService in real-time updaters #6018
Closed
Conversation
This file contains 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
vpaturet
added
Technical Debt
Real-Time Update
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6018 +/- ##
=============================================
- Coverage 69.73% 69.73% -0.01%
- Complexity 17314 17318 +4
=============================================
Files 1960 1960
Lines 74267 74274 +7
Branches 7603 7603
=============================================
+ Hits 51793 51795 +2
- Misses 19831 19839 +8
+ Partials 2643 2640 -3 ☔ View full report in Codecov by Sentry. |
|
Moved back to draft and attempting to provide a better implementation |
|
Superseded by #6029 |
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.
Summary
Real-time updaters use instances of
TransitServiceto look up transit data. Currently these instances are created at construction time.This is problematic since
TransitServiceinstances maintain a reference to a frozen version of theTimetableSnapshotthat is not updated after construction.As of today this does not cause any issue, since the real-time updaters update directly the collections in
TransitModelIndex, and theTransitServicelooks up data inTransitModelIndex.However #6007 changes this behavior by storing the real-time updates in indexes in
TimetableSnapshot, making previous real-time updates invisible from the real-time updaters.Among other regressions, this prevents the
EntityResolverfrom identifying already created extra journeys, and leads to the creation of duplicated trips.This PR refactors the real-time updaters so that they use
TransitServiceinstances that refer to theTimetableSnapshotbuffer, thus giving access to all applied real-time updates (including uncommitted ones).This PR is a prerequisite for #6007
Note: the refactoring consists in:
TransitServiceinstead of aTransitModel.UpdaterConfiguratorso that it injects in the updaters aTransitServicethat points to the TimetableSnapshot buffer.Note: This design is fragile, since there is no way to guarantee that the
TransitServicethat points to the buffer will be used only from the GraphWriter thread (this is however the case in this PR) . Ideally this should be enforced. The fact that theSiriFuzzyTripMatcheris initialized statically makes things more difficult.Issue
No
Unit tests
Added unit test.
Updated unit tests
Documentation
No