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.

Sign up for GitHub

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

Jump to bottom

Fix concurrency issue with NodePeer update #8851

Merged
merged 60 commits into from
Jun 3, 2024
Merged

Fix concurrency issue with NodePeer update #8851

merged 60 commits into from
Jun 3, 2024

Conversation

shubham3121
Copy link
Member

@shubham3121 shubham3121 commented May 23, 2024 •
edited by khoaguin
Loading

Description

  • Now, we are running peers health check periodically in background task (Peer health checks #8679). This PR tries to get the list of online domains using the results of this peers health check feature instead of trying to login the domains as guest clients to check if they are online every time we call sy.domains.
  • Fix concurrency issues when updating the same objects in the document store
  • Using ParitialSyftObject for update

khoaguin and others added 30 commits May 9, 2024 11:03
@khoaguin
[syft/network] getting online networks for python gateway node
355e513
@khoaguin @shubham3121
[syft/network] when checking for online domains, if not able to creat…
9316f69 
…e a guess client at a gateway, just ignore it and move on to another one

Co-authored-by: Shubham Gupta <[email protected]>
@khoaguin
[syft/network] on getting online domains by checking `domain.ping_sta…
7fe68f9 
…tus` for `sy.domains`
@khoaguin
[syft/network] add total line to show number of online networks / all…
25eaaee 
… networks for `sy.gateways`
@khoaguin
[syft/network] - update ping url for networks without frontend
eddc66f 
- improve repr for domain and network registry to show online domains and networks
@khoaguin
[syft/network] integrating background_tasks into Orchestra.launch
8182a14 
- `sy.gateways` and `sy.domains` work with python webserver nodes
@khoaguin
Merge branch 'dev' into peer-health-check-online-domains
8268c29
@khoaguin @shubham3121
[syft/network] pass background_tasks=True to worker_class
da596ff 
- add a wait in `gateway_local_test` before checking for online domains

Co-authored-by: Shubham Gupta <[email protected]>
@khoaguin
[test/integration] add removing peers to the beginning of some tests
71e4a34 
- add some wait times before checking node connection status
@khoaguin
[syft/network] revert back to use ping_status instead of `ping_stat…
0f89b8b 
…us.value` for `NodePeer` repr
@khoaguin
[test/integration] add remove existing peers before testing for `test…
744a289 
…_delete_route_on_peer`
@khoaguin
Merge branch 'dev' into peer-health-check-online-domains
5fd4c31
@khoaguin
[test/integration] trimming down network/gateway_test
c3e0e62
@khoaguin
[test/integration] add some waiting times for local gateway tests bef…
c89d671 
…ore checking for peer connection status
@khoaguin
Merge branch 'dev' into peer-health-check-online-domains
243ca68
@khoaguin
[test/integration] update gateway tests
3782033
@khoaguin
[test/integration] update gateway k8s tests
1545ec3
@khoaguin
Merge branch 'dev' into peer-health-check-online-domains
b2b057f
@khoaguin
[syft/network] - pick the highest priority route to be the oldest by …
2c45fba 
…default

- update k8s integration tests accordingly
- move skip test to the test file rather than in `tox.ini`
@khoaguin
Merge branch 'dev' into peer-health-check-online-domains
eaf59b5
@khoaguin
[syft/network] PeerHealthCheckTask.peer_route_heathcheck now only
a6cd9c9 
updates ping status related fields (in backgroudn threads) instead of
the whole peer object to avoid outdated node routes
(and possibly also other fields) issues
@khoaguin
Merge branch 'dev' into peer-health-check-online-domains
e20ff1f
@khoaguin
fix linting
231bcbf
@khoaguin
Merge branch 'dev' into peer-health-check-online-domains
095a235
@khoaguin
[syft/network] return Err if the returned node peer is None
342439f
@khoaguin
[test/integration] allow running gateway_local_test.py in `syft.tes…
1384e0e 
…t.integration`
@khoaguin
[test/integration] add @pytest.mark.network for tests in `gateway_t…
2dd8873 
…est.py`
@khoaguin
Merge branch 'dev' into peer-health-check-online-domains
821efef
@khoaguin
Merge branch 'dev' into peer-health-check-online-domains
abee20d
@rasswanth-s
add a try catch block to catch invalid api registry entries
7507fff
@khoaguin khoaguin changed the title [WIP] Fix concurrency issue with NodePeer update Fix concurrency issue with NodePeer update May 23, 2024
@khoaguin khoaguin mentioned this pull request May 24, 2024
Closed
@khoaguin
Copy link
Member

image

@khoaguin khoaguin force-pushed the node-peer-partial-update branch from 5387757 to cd44055 Compare May 28, 2024 13:08
khoaguin and others added 5 commits June 3, 2024 13:09
@shubham3121 shubham3121 merged commit 1239eed into dev Jun 3, 2024
26 checks passed
@shubham3121 shubham3121 deleted the node-peer-partial-update branch June 3, 2024 13:07
@shubham3121
Copy link
Member Author

Nice work @khoaguin !!! 🎉 🔥 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
0.8.7 Yellow Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shubham3121 @khoaguin @madhavajay @rasswanth-s