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

refactor: Make some improvements to how often/when we announce a group #2348

Merged
merged 1 commit into from
Nov 25, 2022

Conversation

JFreegman
Copy link
Member

@JFreegman JFreegman commented Oct 8, 2022

Instead of announcing a group whenever our connection status changes, including when we gain or lose a TCP relay connection, we now only announce when our UDP status changes, or if our previously announced TCP relay has gone offline. We also refresh our announcement every 20 minutes regardless of any connection changes (this is to rotate the TCP relay in case the chosen one is somehow broken).

This change should vastly reduce the amount of unnecessary DHT traffic related to group announcements.


This change is Reviewable

@JFreegman JFreegman added the refactor Refactoring production code, eg. renaming a variable, not affecting semantics label Oct 8, 2022
@JFreegman JFreegman added this to the v0.2.19 milestone Oct 8, 2022
@JFreegman JFreegman force-pushed the ngc_announce_fix branch 2 times, most recently from 8b56a49 to 1585048 Compare October 12, 2022 17:57
Copy link

@sudden6 sudden6 left a comment

Choose a reason for hiding this comment

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

Generally looks good, just some documentation stuff

toxcore/group_chats.c Outdated Show resolved Hide resolved
toxcore/group_chats.c Show resolved Hide resolved
@JFreegman JFreegman force-pushed the ngc_announce_fix branch 9 times, most recently from b674660 to a8b0516 Compare November 25, 2022 15:31
Instead of announcing a group whenever our connection status changes,
including when we gain or lose a TCP relay connection, we now only
when our UDP status changes, or if our previously announced
relay has gone offline. We also refresh our announcement
20 minutes regardless of any connection changes.

change should vastly reduce the amount of unnecessary DHT
traffic related to group announcements.
@codecov
Copy link

codecov bot commented Nov 25, 2022

Codecov Report

Merging #2348 (7ee7720) into master (b8aa21c) will decrease coverage by 0.04%.
The diff coverage is 73.33%.

@@            Coverage Diff             @@
##           master    #2348      +/-   ##
==========================================
- Coverage   78.13%   78.08%   -0.05%     
==========================================
  Files         141      141              
  Lines       31108    31114       +6     
==========================================
- Hits        24306    24296      -10     
- Misses       6802     6818      +16     
Impacted Files Coverage Δ
toxcore/Messenger.c 82.56% <60.00%> (-0.08%) ⬇️
toxcore/group_chats.c 68.85% <75.00%> (-0.43%) ⬇️
toxcore/TCP_connection.c 70.54% <100.00%> (+0.37%) ⬆️
auto_tests/tox_many_test.c 98.18% <0.00%> (-1.82%) ⬇️
toxcore/ping.c 86.92% <0.00%> (-1.31%) ⬇️
toxcore/net_crypto.c 87.15% <0.00%> (-0.23%) ⬇️
toxav/toxav.c 69.08% <0.00%> (-0.15%) ⬇️
toxcore/DHT.c 84.82% <0.00%> (-0.08%) ⬇️
auto_tests/group_sync_test.c 84.65% <0.00%> (ø)
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@JFreegman JFreegman merged commit 7ee7720 into TokTok:master Nov 25, 2022
@JFreegman JFreegman deleted the ngc_announce_fix branch November 25, 2022 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring production code, eg. renaming a variable, not affecting semantics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants