feat: network observer cover from not connected and blocked to connected (WPB-11200) #3057

yamilmedina · 2024-10-11T17:50:45Z

WPB-11200 [Android] investigate the posintial of connectivity observer blocking sync

PR Submission Checklist for internal contributors

The PR Title
- conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
- contains a reference JIRA issue number like SQPIT-764
- answers the question: If merged, this PR will: ... ³
The PR Description
- is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

There is one case that seems to be not covered by the network state observer being:

Not connected -> to Unblocked = Connected

Causes (Optional)

Potentially hanging the state until a new connected state is triggered.

Solutions

Covering all grounds by adding this case and some logs to help debugging in datadog

Testing

Test Coverage (Optional)

I have added automated test to this contribution

Notes (Optional)

For now I'm testing this in an local build, to gather logs and monitor improvements

PR Post Submission Checklist for internal contributors (Optional)

Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References

https://sparkbox.com/foundry/semantic_commit_messages
https://github.com/wireapp/.github#usage
E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

github-actions · 2024-10-11T18:01:01Z

Test Results

3 235 tests +16 3 129 ✅ +16 3m 58s ⏱️ +16s
552 suites ± 0 106 💤 ± 0
552 files ± 0 0 ❌ ± 0

Results for commit c3b21fd. ± Comparison against base commit fc3f3e3.

This pull request removes 1 and adds 17 tests. Note that renamed tests count towards both.

com.wire.kalium.logic.data.conversation.ConversationRepositoryTest ‑ givenAGroupConversationHasNewMessages_whenObservingConversationListDetails_ThenCorrectlyGetUnreadMessageCount[jvm]

com.wire.kalium.logic.data.conversation.ConversationMapperTest ‑ givenArchivedConversationWithDraftAndLastMessage_whenMappingFromDAODetailsWithEventsToModel_thenReturnProperLastMessage[jvm]
com.wire.kalium.logic.data.conversation.ConversationMapperTest ‑ givenConversationWithDraftAndLastMessage_whenMappingFromDAODetailsWithEventsToModel_thenReturnProperLastMessage[jvm]
com.wire.kalium.logic.data.conversation.ConversationMapperTest ‑ givenConversationWithLastMessage_whenMappingFromDAODetailsWithEventsToModel_thenReturnProperLastMessage[jvm]
com.wire.kalium.logic.data.conversation.ConversationMapperTest ‑ givenConversationWithNoLastMessageAndDraft_whenMappingFromDAODetailsWithEventsToModel_thenReturnProperLastMessage[jvm]
com.wire.kalium.logic.data.conversation.ConversationRepositoryTest ‑ givenAConversationHasLastMessageAndDraft_whenObservingConversationListDetails_ThenCorrectlyGetLastMessage[jvm]
com.wire.kalium.logic.data.conversation.ConversationRepositoryTest ‑ givenAOneToOneConversationHasNewMessages_whenObservingConversationListDetails_ThenCorrectlyGetUnreadMessageCount[jvm]
com.wire.kalium.persistence.dao.ConversationDAOTest ‑ givenAGroupConvWhichSelfUserLeft_whenGettingAllConversationsWithEventsAndOnlyEnabledInteractions_thenDoNotReturnIt[jvm]
com.wire.kalium.persistence.dao.ConversationDAOTest ‑ givenAOneOneConvWithDeletedUser_whenGettingAllConversationsWithEventsAndOnlyEnabledInteractions_thenDoNotReturnIt[jvm]
com.wire.kalium.persistence.dao.ConversationDAOTest ‑ givenArchiveConversationWithUnreadMessageAndDraft_whenGettingAllConversationsWithEvents_thenShouldReturnCorrectValues[jvm]
com.wire.kalium.persistence.dao.ConversationDAOTest ‑ givenConnectionRequest_whenGettingAllConversationsWithEventsAndOnlyEnabledInteractions_thenDoNotReturnIt[jvm]
…

♻️ This comment has been updated with latest results.

github-actions · 2024-10-11T18:06:57Z

Bencher Report

Branch	feat/network-observer
Testbed	ubuntu-latest

⚠️ WARNING: The following Measure does not have a Threshold. Without a Threshold, no Alerts will ever be generated!
Latency
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results

Benchmark	Latency	nanoseconds (ns)
com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInFiles	📈 view plot ⚠️ NO THRESHOLD	680,129.39
com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInMemory	📈 view plot ⚠️ NO THRESHOLD	324,488,051.38
com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.messageInsertionBenchmark	📈 view plot ⚠️ NO THRESHOLD	934,174,437.83
com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.queryMessagesBenchmark	📈 view plot ⚠️ NO THRESHOLD	21,192,383.44

🐰 View full continuous benchmarking report in Bencher

codecov-commenter · 2024-10-11T18:17:59Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.23%. Comparing base (fc3f3e3) to head (c3b21fd).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3057      +/-   ##
===========================================
- Coverage    52.43%   52.23%   -0.20%     
===========================================
  Files         1309     1314       +5     
  Lines        50337    50951     +614     
  Branches      4671     4724      +53     
===========================================
+ Hits         26394    26616     +222     
- Misses       22055    22440     +385     
- Partials      1888     1895       +7

see 24 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc3f3e3...c3b21fd. Read the comment docs.

datadog-wireapp · 2024-10-11T18:18:31Z

Datadog Report

Branch report: feat/network-observer
Commit report: 45bdff5
Test service: kalium-jvm

✅ 0 Failed, 3129 Passed, 106 Skipped, 15.73s Total Time

damian-kaczmarek

I was able to produce this strange state where there is no connection but app do not register that. @yamilmedina Do you think this is a part of this problem or separate issue?

How to reproduce:

airplane mode on
wait a little bit
airplane mode off
quickly airplane mode on

strange_network.mp4

yamilmedina · 2024-10-14T09:49:23Z

I was able to produce this strange state where there is no connection but app do not register that. @yamilmedina Do you think this is a part of this problem or separate issue?

How to reproduce:

airplane mode on

wait a little bit

airplane mode off

quickly airplane mode on

strange_network.mp4

Thanks for the repro info, indeed this case we are trying to cover from many sides, so it is somewhat related.
I will take a look and see if I can include the fix on this PR or maybe create a new ticket if there is more work to do, since this spike would be an option too :D

sonarqubecloud · 2024-10-14T16:45:12Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

yamilmedina · 2024-10-14T17:18:46Z

I was able to produce this strange state where there is no connection but app do not register that. @yamilmedina Do you think this is a part of this problem or separate issue?

How to reproduce:

airplane mode on

wait a little bit

airplane mode off

quickly airplane mode on

strange_network.mp4

Ok, I investigated further, and the issue can happen after the second time in airplane mode (it does no matter if it's quick or not, at least for me). In the worst case, this gets short-circuited after ~20 seconds, when the websocket does a ping, this happens since we delay when no connection for the incremental sync to happen.

I will create a ticket, to see how we can improve this state change, for the scope of this ticket at least is not relevant, since is related to the other case (from no network to connected) and blocking the sync because of it.

Anyway, this helped me to take a look at some buffer config we could use for this PR and having a quicker reaction back to connected state :D

damian-kaczmarek

Edit: didn't noticed your comment, nvm
~~@yamilmedina I was still able to reproduce it, do we want separate issue for that?~~

yamilmedina · 2024-10-15T08:47:11Z

Created a ticket for this other issue here:
https://wearezeta.atlassian.net/browse/WPB-11568

yamilmedina added 3 commits October 10, 2024 14:22

feat: trying to fix network observer

287a7ec

chore: add logs for block state

306dca6

chore: tests

b285d2a

yamilmedina self-assigned this Oct 11, 2024

github-actions bot added 👕 size: S type: feature ✨ labels Oct 11, 2024

chore: tests

d6c28dc

yamilmedina requested review from a team, m-zagorski, alexandreferris, borichellow, MohamadJaara, damian-kaczmarek, Garzas and saleniuk and removed request for a team October 11, 2024 17:51

Garzas approved these changes Oct 14, 2024

View reviewed changes

damian-kaczmarek suggested changes Oct 14, 2024

View reviewed changes

chore: use buffer conflated for network states

beac183

yamilmedina requested a review from damian-kaczmarek October 14, 2024 16:04

chore: detekt

c3b21fd

damian-kaczmarek reviewed Oct 15, 2024

View reviewed changes

damian-kaczmarek approved these changes Oct 15, 2024

View reviewed changes

yamilmedina added this pull request to the merge queue Oct 15, 2024

Merged via the queue into develop with commit b6d117d Oct 15, 2024
22 checks passed

yamilmedina deleted the feat/network-observer branch October 15, 2024 09:12

echoes-hq bot added the echoes: product-roadmap Work aligned with the customer-announced roadmap, targeting a specific release date. label Nov 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: network observer cover from not connected and blocked to connected (WPB-11200) #3057

feat: network observer cover from not connected and blocked to connected (WPB-11200) #3057

yamilmedina commented Oct 11, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Oct 11, 2024 •

edited

Loading

github-actions bot commented Oct 11, 2024 •

edited

Loading

codecov-commenter commented Oct 11, 2024 •

edited

Loading

datadog-wireapp bot commented Oct 11, 2024 •

edited

Loading

damian-kaczmarek left a comment

yamilmedina commented Oct 14, 2024

sonarqubecloud bot commented Oct 14, 2024

yamilmedina commented Oct 14, 2024 •

edited

Loading

damian-kaczmarek left a comment •

edited

Loading

yamilmedina commented Oct 15, 2024

feat: network observer cover from not connected and blocked to connected (WPB-11200) #3057

feat: network observer cover from not connected and blocked to connected (WPB-11200) #3057

Conversation

yamilmedina commented Oct 11, 2024 • edited by github-actions bot Loading

PR Submission Checklist for internal contributors

What's new in this PR?

Issues

Causes (Optional)

Solutions

Testing

Test Coverage (Optional)

Notes (Optional)

PR Post Submission Checklist for internal contributors (Optional)

PR Post Merge Checklist for internal contributors

References

github-actions bot commented Oct 11, 2024 • edited Loading

Test Results

github-actions bot commented Oct 11, 2024 • edited Loading

Bencher Report

codecov-commenter commented Oct 11, 2024 • edited Loading

Codecov Report

datadog-wireapp bot commented Oct 11, 2024 • edited Loading

Datadog Report

damian-kaczmarek left a comment

Choose a reason for hiding this comment

yamilmedina commented Oct 14, 2024

sonarqubecloud bot commented Oct 14, 2024

Quality Gate passed

yamilmedina commented Oct 14, 2024 • edited Loading

damian-kaczmarek left a comment • edited Loading

Choose a reason for hiding this comment

yamilmedina commented Oct 15, 2024

yamilmedina commented Oct 11, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Oct 11, 2024 •

edited

Loading

github-actions bot commented Oct 11, 2024 •

edited

Loading

codecov-commenter commented Oct 11, 2024 •

edited

Loading

datadog-wireapp bot commented Oct 11, 2024 •

edited

Loading

yamilmedina commented Oct 14, 2024 •

edited

Loading

damian-kaczmarek left a comment •

edited

Loading