refactor: move sync logic out of Client #12841
Conversation
Longarithm
force-pushed
the
sync-manager
branch
from
96bef52 to
37769e5
Compare
chain/client/src/sync/handler.rs
Outdated
| ) -> Result<Vec<(CryptoHash, PeerId)>, near_chain::Error> { | ||
| let now = self.clock.now_utc(); | ||
|
|
||
| let mut have_all = true; |
There was a problem hiding this comment.
nit: this used to be returned so the caller can return from handle_sync_needed(), but now it's just logged at the bottom, so could just get rid of that and log whatever you want, maybe blocks_to_request.len() or something
chain/client/src/sync/handler.rs
Outdated
| ); | ||
| let state_sync_result = unwrap_and_report_state_sync_result!(state_sync_result); | ||
| match state_sync_result { | ||
| StateSyncResult::InProgress => return None, |
There was a problem hiding this comment.
nit: maybe just if == (might have to add Eq, PartialEq) instead of the old code's match
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12841 +/- ##
==========================================
- Coverage 70.39% 70.39% -0.01%
==========================================
Files 848 851 +3
Lines 174091 174195 +104
Branches 174091 174195 +104
==========================================
+ Hits 122548 122617 +69
- Misses 46296 46334 +38
+ Partials 5247 5244 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Reducing number of fields in
Clientfrom 41 to 36 and number of lines in client_actor.rs from 2280 to 1974.There are couple fields clearly related to sync, so I move them to
SyncHandler. Its main purpose is to hide sync implementation details and executehandle_sync_neededwhich is called, well, when sync is needed.Small caveat was that sometimes an action is needed on Client side - like, request some blocks for state sync. I add
SyncHandlerRequestfor that, hiding sync logic still seems much better.Later it should be possible to hide
catchup_state_syncsand related fields as well. We can also hide all fields insideSyncHandlerif needed.Diff of movement of the largest functions for convenience: 12841.txt