fix_: use 24h batches to retrieve message history #21674
Conversation
Jenkins Builds
|
88% of end-end tests have passedExpected to fail tests (1)Click to expandClass TestCommunityMultipleDeviceMerged:
Passed tests (7)Click to expandClass TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
|
|
@richard-ramos |
100% of end-end tests have passedPassed tests (8)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestWalletMultipleDevice:
|
80% of end-end tests have passedFailed tests (10)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestFallbackMultipleDevice:
Expected to fail tests (1)Click to expandClass TestCommunityMultipleDeviceMerged:
Passed tests (44)Click to expandClass TestWalletMultipleDevice:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
|
9d according to this setting: https://github.com/status-im/status-go/blob/develop/appdatabase/migrations/sql/1682073779_settings_table_remove_latest_derived_path_column.up.sql#L55 It will not load the 9d immediatly though as it will iterate over the result set day by day from most recent to earlier. Also, perhaps this is a parameter we could change to reduce the load on the storenodes to maybe 3d instead? wdyt @fryorcraken? I imagine this should reduce the number of requests done to a storenode when joining a community to ~1/3 of the number of requests we do now depending on how active a community is. |
|
I'll comment one by one For today stat the test was failed 3 times and passed 5 times (all that on staging fleet) and it seems unstable on both fleets - so sometimes it is restored, sometimes not.
What we should exactly ensure here and in what fleet? 2) Curated communities 3) Fetching history in communities
It has never been working on mobile, so frankly I do not really know how to test it. When you join for example Status community from mobile, only 24h were fetched. And we do not have an option in nightly of to fetch more history, and usually even the one for 24h is very incomplete (as it can be seen from #21581) which is totally valid issue. So basically what I can test now for sure that new communities that are created on |
|
So we made a small investigation of how app behaves on PR and nightly builds (thanks @Horupa-Olena), the comparison spreadsheet is here Based on the comparison table provided in the document, here are the conclusions regarding key differences between the PR and Nightly builds: General Observations:
Channel-Specific Differences:
Additional Notes:
Conclusion:
But it all might be a coincidence |
76% of end-end tests have passedFailed tests (12)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestFallbackMultipleDevice:
Expected to fail tests (1)Click to expandClass TestCommunityMultipleDeviceMerged:
Passed tests (42)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestWalletOneDevice:
Class TestDeepLinksOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
|
That the backup feature still works. This should work on any fleet.
Yes
Perhaps this is something that should be confirmed on the PR created for desktop? cc: @anastasiyaig Overall the behaviour should be the same, except that on the background the queries are splitted into 24h batches which should help reduce the load on the storenodes as well as improve the response times for the store queries, which seems to be the case, based on the comment before this! For the cases that sometimes work / sometimes doesn't, it seems to me that this is likely that some storenode is missing messages or maybe it wasn't even possible to publish the message containing the backup. I wonder what's the experience like with this PR on desktop? Thank you, @churik |
|
sadly i did not try yet , pretty loaded with tasks for 2.32 . i will be back to it, there is a desktop pr, which i am planning to try tomorrow @richard-ramos |
|
some context for prod fleet: tests are passing here status-im/status-desktop#16813 i need to check staging and backup manually, will reply in desktop pr |
|
I cannot really prove that this is actually PR issue, but worth checking ISSUE 1 sometimes messages are not fetched from history nodes in 1-1 chat when the sender is offline Reproducibility:
Prerequisites: sender and receiver are mutual contacts Steps:
Expected result: messages will be fetched from history node in max 2 mins after the receiver goes back online Sender logs: logs.zip Receiver logs: Status-debug-logs.zip |
|
apart from #21674 (comment) and taking into account #21674 (comment) I can confirm that:
So if you are 100% sure that the message fetching in 1-1 chat is not getting worse in this PR (meaning: #21674 (comment) is not related to PR), you have a green light from mobile side. |
|
Looking at the logs I'm thinking this failure to retrieve messages is not related to the PR. This PR what it does is to change the start/end time of the queries ONLY if they exceed 24h, which is not the case for the queries being done when going from offline to online. |
|
@richard-ramos if you can upgrade version on mobile client to include your changes, I'd appreciate that |
richard-ramos
force-pushed
the
fix/24h-batches
branch
from
4ac1616 to
b9af66f
Compare
|
Done, in last commit i updated the status-go version to the commit from |
|
thanks! it seems that your commit is already included to current mobile version in nightly, tests confirmed that, so I'm closing current PR and initial issue |
Instead of retrieving 30d of messages, split the request in 24h batches.
Requires: status-im/status-go#6115
To test this:
Ensure that profiles restore from backup successfully, as well as curated communities, and message history older than 24h