Skip to content

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-legacy#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.

6 participants