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

fix: use bad peer removal logic only for lightpush and filter and option to restart missing message verifier #1244

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

chaitanyaprem
Copy link
Collaborator

@chaitanyaprem chaitanyaprem commented Oct 18, 2024

Description

Bunch of fixes and changes that help with offline state.

Changes

  • fix: filter stats mismatch and add bad peer check for light mode #1241 had some side-effects where-in store nodes were being removed from peerstore which we do not want because in status store nodes are fixed (i.e fleet nodes).
  • Apart from that i had noticed that when node is offline, connections are being triggered which result in failures and many good peers such as fleet nodes are getting removed from peer-store.
  • Added a more strict check and now migrated the bad peer removal logic to only be done from lightpush and filter and that too based on specific errors to avoid such side-effects.
  • Found an issue with offline to online state change handling in filter-manager where filter ping to existing subs will never happen.
  • Added a check to filter ping error handling in case node is already offline so that sub are not cleared
  • Added functionality to specify preferred-peers for filter. One of the peers used for filter subscription out of n will be randomly chosen from list of preferred peers.
  • Added a fix for missing messages to stop it when node goes offline in order to reduce backlog it accumulates and never being able to catch-up.

Tests

Being dogfooded status-im/status-mobile#21172 for various network disconnection/switch scenarios.

@chaitanyaprem chaitanyaprem changed the title not to merge: pr to test filter changes fix: pr to test filter changes (not to merge) Oct 18, 2024
@status-im-auto
Copy link

status-im-auto commented Oct 18, 2024

Jenkins Builds

Click to see older builds (13)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 8842d00 #1 2024-10-18 10:54:54 ~1 min unknown 📄log
✔️ 103d26c #2 2024-10-23 11:19:40 ~2 min unknown 📄log
✔️ 4e3d73c #3 2024-10-24 09:38:27 ~2 min unknown 📄log
✔️ be7eb68 #4 2024-10-24 09:57:25 ~3 min unknown 📄log
✔️ 5322d34 #5 2024-10-24 10:07:16 ~2 min unknown 📄log
✔️ 484e141 #6 2024-10-29 10:45:05 ~2 min unknown 📄log
✔️ 9cdf9ec #7 2024-10-29 11:40:07 ~2 min unknown 📄log
✔️ 213d382 #8 2024-11-05 17:48:44 ~2 min unknown 📄log
✔️ aadfbd6 #9 2024-11-09 10:32:46 ~2 min unknown 📄log
✔️ 26bbb69 #10 2024-11-29 04:53:40 ~1 min unknown 📄log
✔️ 313c6bf #11 2024-12-02 01:49:54 ~2 min unknown 📄log
✔️ 5244e7e #12 2024-12-03 07:50:45 ~2 min unknown 📄log
✔️ 299c471 #13 2024-12-03 08:16:05 ~2 min unknown 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 692b862 #14 2024-12-03 16:51:08 ~2 min unknown 📄log
✔️ e0a9ae0 #15 2024-12-04 12:05:46 ~2 min unknown 📄log

@chaitanyaprem chaitanyaprem force-pushed the test/filter-bad-peers branch 3 times, most recently from be7eb68 to 5322d34 Compare October 24, 2024 10:04
@richard-ramos richard-ramos force-pushed the master branch 3 times, most recently from c6c6479 to 6bdf125 Compare October 24, 2024 18:32
@chaitanyaprem chaitanyaprem changed the title fix: pr to test filter changes (not to merge) fix: add option to specify preferred peers for filter and use bad peer removal logic only for lightpush and filter Nov 29, 2024
@chaitanyaprem chaitanyaprem marked this pull request as ready for review November 29, 2024 04:56
@chaitanyaprem chaitanyaprem force-pushed the test/filter-bad-peers branch 2 times, most recently from 299c471 to 692b862 Compare December 3, 2024 16:48
@chaitanyaprem chaitanyaprem changed the title fix: add option to specify preferred peers for filter and use bad peer removal logic only for lightpush and filter fix: use bad peer removal logic only for lightpush and filter and provide option to restart missing message verifier Dec 4, 2024
Copy link
Contributor

@kaichaosun kaichaosun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fryorcraken
Copy link
Collaborator

You crossed out

Added functionality to specify preferred-peers for filter. One of the peers used for filter subscription out of n will be randomly chosen from list of preferred peers.

But it is not reflected in PR title. Is that in or out? (hopefully out)

@chaitanyaprem
Copy link
Collaborator Author

But it is not reflected in PR title. Is that in or out? (hopefully out)

I did update the PR title to not include it :) Weird if you are still seeing it in the title .

Current title is fix: use bad peer removal logic only for lightpush and filter and provide option to restart missing message verifier

Could be some state update issue in github?

@chaitanyaprem chaitanyaprem changed the title fix: use bad peer removal logic only for lightpush and filter and provide option to restart missing message verifier fix: use bad peer removal logic only for lightpush and filter and option to restart missing message verifier Dec 5, 2024
@chaitanyaprem chaitanyaprem merged commit 809dba5 into master Dec 9, 2024
13 checks passed
@chaitanyaprem chaitanyaprem deleted the test/filter-bad-peers branch December 9, 2024 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants