Stop the rate limiter from being its own amplifier
The control-message pacing added two commits ago logged one WARN line per refused packet, which is a thousand formatted strings and a thousand stdout writes per second for a client sending a thousand -- the exact cost the limiter was added to bound, moved from the disk write to the log. One line per burst now, re-armed when the peer behaves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
7b710471df
commit
54086b5604
2 files changed
+57
-1
No files matched your search
@@ -38,6 +38,11 @@ var _snapshot_phase: int = 0
|
||||
var _tick: int = 0
|
||||
## Tick each peer was last allowed a control message on.
|
||||
var _peer_control_tick: Dictionary[int, int] = {}
|
||||
## Peers already told off for going too fast. Without this the refusal logs one
|
||||
## line per packet, which turns the limiter into the amplification it exists to
|
||||
## prevent -- a client sending a thousand a second would get a thousand lines
|
||||
## formatted and written. One line per burst, cleared when the peer behaves.
|
||||
var _peer_control_warned: Dictionary[int, bool] = {}
|
||||
|
||||
|
||||
func _ready() -> void:
|
||||
@@ -205,6 +210,7 @@ func _forget_peer(peer_id: int) -> void:
|
||||
peer_accounts.erase(peer_id)
|
||||
peer_characters.erase(peer_id)
|
||||
_peer_control_tick.erase(peer_id)
|
||||
_peer_control_warned.erase(peer_id)
|
||||
_broadcast_roster()
|
||||
|
||||
|
||||
@@ -427,9 +433,14 @@ func instance_of(peer_id: int) -> Instance:
|
||||
func _accept_control(peer_id: int) -> bool:
|
||||
var last: int = _peer_control_tick.get(peer_id, -CONTROL_COOLDOWN_TICKS)
|
||||
if _tick - last < CONTROL_COOLDOWN_TICKS:
|
||||
GameLog.warn("server", "peer %d control message dropped, too fast" % peer_id)
|
||||
if not _peer_control_warned.get(peer_id, false):
|
||||
_peer_control_warned[peer_id] = true
|
||||
GameLog.warn("server",
|
||||
"peer %d is sending control messages faster than %d ticks apart"
|
||||
% [peer_id, CONTROL_COOLDOWN_TICKS])
|
||||
return false
|
||||
_peer_control_tick[peer_id] = _tick
|
||||
_peer_control_warned.erase(peer_id)
|
||||
return true
|
||||
|
||||
|
||||
|
||||
@@ -306,3 +306,48 @@ func test_clamping_the_muzzle_does_not_swallow_the_shot() -> void:
|
||||
# a blocked muzzle falls back to the shooter's centre, it does not cancel.
|
||||
assert_eq(int(_fire_flush_against_the_wall(6)["fired"]), 7,
|
||||
"six Doubleshots is seven projectiles, wall or no wall")
|
||||
|
||||
|
||||
# --- Control-message pacing -------------------------------------------------
|
||||
# Selecting or creating a character rewrites the whole store to disk, re-places
|
||||
# the player and broadcasts a roster to everyone, all off one small reliable
|
||||
# packet. The pacing below is what stops a client turning that into a synchronous
|
||||
# disk write it controls the rate of.
|
||||
|
||||
func _runtime() -> ServerRuntime:
|
||||
# Not parented: _ready() would open a lobby and load a store, and neither is
|
||||
# involved in deciding whether a message arrived too soon.
|
||||
return autofree(ServerRuntime.new()) as ServerRuntime
|
||||
|
||||
|
||||
func test_control_messages_are_paced_per_peer() -> void:
|
||||
var srv := _runtime()
|
||||
assert_true(srv._accept_control(PEER), "the first one is always allowed")
|
||||
assert_false(srv._accept_control(PEER), "the second on the same tick is not")
|
||||
srv._tick += ServerRuntime.CONTROL_COOLDOWN_TICKS - 1
|
||||
assert_false(srv._accept_control(PEER), "one tick short is still too soon")
|
||||
srv._tick += 1
|
||||
assert_true(srv._accept_control(PEER), "and the cooldown does expire")
|
||||
|
||||
|
||||
func test_one_peer_flooding_does_not_gag_another() -> void:
|
||||
var srv := _runtime()
|
||||
assert_true(srv._accept_control(1))
|
||||
assert_false(srv._accept_control(1))
|
||||
assert_true(srv._accept_control(2), "pacing is per peer, not global")
|
||||
|
||||
|
||||
func test_a_refused_flood_is_logged_once_and_not_once_per_packet() -> void:
|
||||
# The limiter must not become the amplification it exists to prevent: a
|
||||
# client sending a thousand a second would otherwise have the server format
|
||||
# and write a thousand lines.
|
||||
var srv := _runtime()
|
||||
assert_true(srv._accept_control(PEER))
|
||||
for _i in 500:
|
||||
assert_false(srv._accept_control(PEER))
|
||||
assert_true(srv._peer_control_warned.get(PEER, false),
|
||||
"the first refusal of a burst is worth saying")
|
||||
srv._tick += ServerRuntime.CONTROL_COOLDOWN_TICKS
|
||||
assert_true(srv._accept_control(PEER))
|
||||
assert_false(srv._peer_control_warned.has(PEER),
|
||||
"behaving again re-arms the warning for the next burst")
|
||||
Reference in new issue
Block a user