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: [Network] Prevent stalling save game transfer when compression is slow #9106

Merged
merged 1 commit into from Apr 25, 2021

Conversation

@rubidium42
Copy link
Contributor

@rubidium42 rubidium42 commented Apr 25, 2021

Motivation / Problem

If the threaded savegame logic yields packets slow enough, e.g. 1 every tick, and the network is fast enough, i.e. it can handle 33 packets / second (~400 kbps or ~8 Mbps with #9099), then sent_packets would be doubled every tick. From an initial value of 4
it would take 29 ticks to get to 2**31 and on tick 30 it would overflow into 0. From that moment on the server cannot send any packets anymore as i < sent_packets would always return false.

Description

Just limit to how far send_packets can be doubled. Similarly, reduce the type size because the number of packets that may be sent per tick becomes quite hilarious quickly.

Limitations

This artificially limits the save game transfer speed to 1400 MiB/s (or 35 GiB/s with #9099 applied). If the saving would be able to saturate the network link, this artificial limit is reached after 91 MiB (or 2 GiB with #9099 applied) of the save game was already sent due to the ramp up from 4 to 32768 packets per second every tick.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')
src/network/network_server.cpp Outdated Show resolved Hide resolved
@rubidium42 rubidium42 force-pushed the broken_savegame_transfer branch from 6263478 to d5eb300 Apr 25, 2021
@rubidium42 rubidium42 force-pushed the broken_savegame_transfer branch from d5eb300 to b0028fb Apr 25, 2021
@rubidium42 rubidium42 force-pushed the broken_savegame_transfer branch from b0028fb to 470ee01 Apr 25, 2021
@rubidium42 rubidium42 changed the title Fix: [Network] Under certain circumstances the 'sent_packets' could overflow... Fix: [Network] Prevent stalling save game transfer when compression is slow Apr 25, 2021
@rubidium42 rubidium42 merged commit 65818db into OpenTTD:master Apr 25, 2021
12 checks passed
@rubidium42 rubidium42 deleted the broken_savegame_transfer branch Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants