Merge branch 'xMAC94x/fixDisconnectMetrics' into 'master'

instead of manually trying to sync disconnect metrics we now pass a Reason to...

See merge request veloren/veloren!2259
This commit is contained in:
Marcel 2021-05-06 11:32:37 +00:00
commit 91f76d3b00
11 changed files with 73 additions and 57 deletions

View File

@ -84,6 +84,7 @@ pub use self::{
Collider, Density, ForceUpdate, Mass, PhysicsState, Pos, PosVelDefer, PreviousPhysCache,
Scale, Sticky, Vel,
},
player::DisconnectReason,
player::Player,
poise::{Poise, PoiseChange, PoiseSource, PoiseState},
projectile::{Projectile, ProjectileConstructor},

View File

@ -5,6 +5,15 @@ use uuid::Uuid;
const MAX_ALIAS_LEN: usize = 32;
#[derive(Debug)]
pub enum DisconnectReason {
Kicked,
NewerLogin,
NetworkError,
Timeout,
ClientRequested,
}
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct Player {
pub alias: String,

View File

@ -4,7 +4,7 @@ use crate::{
self,
invite::{InviteKind, InviteResponse},
item::Item,
Ori, Pos,
DisconnectReason, Ori, Pos,
},
outcome::Outcome,
rtsim::RtSimEntity,
@ -135,7 +135,7 @@ pub enum ServerEvent {
rtsim_entity: Option<RtSimEntity>,
},
CreateWaypoint(Vec3<f32>),
ClientDisconnect(EcsEntity),
ClientDisconnect(EcsEntity, DisconnectReason),
ClientDisconnectWithoutPersistence(EcsEntity),
ChunkRequest(EcsEntity, Vec2<i32>),
ChatCmd(EcsEntity, String),

View File

@ -16,7 +16,6 @@ pub struct Client {
pub participant: Option<Participant>,
pub last_ping: Mutex<f64>,
pub login_msg_sent: AtomicBool,
pub terminate_msg_recv: AtomicBool,
//TODO: improve network crate so that `send` is no longer `&mut self` and we can get rid of
// this Mutex. This Mutex is just to please the compiler as we do not get into contention
@ -68,7 +67,6 @@ impl Client {
participant: Some(participant),
last_ping: Mutex::new(last_ping),
login_msg_sent: AtomicBool::new(false),
terminate_msg_recv: AtomicBool::new(false),
general_stream: Mutex::new(general_stream),
ping_stream: Mutex::new(ping_stream),
register_stream: Mutex::new(register_stream),

View File

@ -2478,7 +2478,10 @@ fn kick_player(
server
.state
.mut_resource::<EventBus<ServerEvent>>()
.emit_now(ServerEvent::ClientDisconnect(target_player));
.emit_now(ServerEvent::ClientDisconnect(
target_player,
common::comp::DisconnectReason::Kicked,
));
Ok(())
}

View File

@ -172,11 +172,16 @@ impl Server {
rtsim_entity,
} => handle_create_ship(self, pos, ship, mountable, agent, rtsim_entity),
ServerEvent::CreateWaypoint(pos) => handle_create_waypoint(self, pos),
ServerEvent::ClientDisconnect(entity) => {
frontend_events.push(handle_client_disconnect(self, entity, false))
ServerEvent::ClientDisconnect(entity, reason) => {
frontend_events.push(handle_client_disconnect(self, entity, reason, false))
},
ServerEvent::ClientDisconnectWithoutPersistence(entity) => {
frontend_events.push(handle_client_disconnect(self, entity, true))
frontend_events.push(handle_client_disconnect(
self,
entity,
common::comp::DisconnectReason::Kicked,
true,
))
},
ServerEvent::ChunkRequest(entity, key) => {

View File

@ -1,7 +1,7 @@
use super::Event;
use crate::{
client::Client, persistence::character_updater::CharacterUpdater, presence::Presence,
state_ext::StateExt, Server,
client::Client, metrics::PlayerMetrics, persistence::character_updater::CharacterUpdater,
presence::Presence, state_ext::StateExt, Server,
};
use common::{
comp,
@ -96,9 +96,20 @@ pub fn handle_exit_ingame(server: &mut Server, entity: EcsEntity) {
}
}
fn get_reason_str(reason: &comp::DisconnectReason) -> &str {
match reason {
comp::DisconnectReason::Timeout => "timeout",
comp::DisconnectReason::NetworkError => "network_error",
comp::DisconnectReason::NewerLogin => "newer_login",
comp::DisconnectReason::Kicked => "kicked",
comp::DisconnectReason::ClientRequested => "client_requested",
}
}
pub fn handle_client_disconnect(
server: &mut Server,
mut entity: EcsEntity,
reason: comp::DisconnectReason,
skip_persistence: bool,
) -> Event {
span!(_guard, "handle_client_disconnect");
@ -112,6 +123,14 @@ pub fn handle_client_disconnect(
// receiving multiple `ServerEvent::ClientDisconnect` messages in a tick
// intended for the same player, so the `None` case here is *not* a bug
// and we should not log it as a potential issue.
server
.state()
.ecs()
.read_resource::<PlayerMetrics>()
.clients_disconnected
.with_label_values(&[get_reason_str(&reason)])
.inc();
if let Some(participant) = client.participant.take() {
let pid = participant.remote_pid();
server.runtime.spawn(
@ -135,7 +154,8 @@ pub fn handle_client_disconnect(
.instrument(tracing::debug_span!(
"client_disconnect",
?pid,
?entity
?entity,
?reason,
)),
);
}

View File

@ -861,7 +861,7 @@ impl Server {
for (_, entity) in (&clients, &entities).join() {
info!("Emitting client disconnect event for entity: {:?}", entity);
let event = if with_persistence {
ServerEvent::ClientDisconnect(entity)
ServerEvent::ClientDisconnect(entity, comp::DisconnectReason::Kicked)
} else {
ServerEvent::ClientDisconnectWithoutPersistence(entity)
};

View File

@ -1,4 +1,4 @@
use crate::{client::Client, metrics::PlayerMetrics};
use crate::client::Client;
use common::{
comp::{ChatMode, Player},
event::{EventBus, ServerEvent},
@ -9,8 +9,7 @@ use common_ecs::{Job, Origin, Phase, System};
use common_net::msg::{
validate_chat_msg, ChatMsgValidationError, ClientGeneral, MAX_BYTES_CHAT_MSG,
};
use specs::{Entities, Join, Read, ReadExpect, ReadStorage};
use std::sync::atomic::Ordering;
use specs::{Entities, Join, Read, ReadStorage};
use tracing::{debug, error, warn};
impl Sys {
@ -19,9 +18,8 @@ impl Sys {
fn handle_general_msg(
server_emitter: &mut common::event::Emitter<'_, ServerEvent>,
entity: specs::Entity,
client: &Client,
_client: &Client,
player: Option<&Player>,
player_metrics: &ReadExpect<'_, PlayerMetrics>,
uids: &ReadStorage<'_, Uid>,
chat_modes: &ReadStorage<'_, ChatMode>,
msg: ClientGeneral,
@ -56,12 +54,10 @@ impl Sys {
},
ClientGeneral::Terminate => {
debug!(?entity, "Client send message to terminate session");
player_metrics
.clients_disconnected
.with_label_values(&["gracefully"])
.inc();
client.terminate_msg_recv.store(true, Ordering::Relaxed);
server_emitter.emit(ServerEvent::ClientDisconnect(entity));
server_emitter.emit(ServerEvent::ClientDisconnect(
entity,
common::comp::DisconnectReason::ClientRequested,
));
},
_ => unreachable!("not a client_general msg"),
}
@ -78,7 +74,6 @@ impl<'a> System<'a> for Sys {
Entities<'a>,
Read<'a, EventBus<ServerEvent>>,
Read<'a, Time>,
ReadExpect<'a, PlayerMetrics>,
ReadStorage<'a, Uid>,
ReadStorage<'a, ChatMode>,
ReadStorage<'a, Player>,
@ -91,16 +86,7 @@ impl<'a> System<'a> for Sys {
fn run(
_job: &mut Job<Self>,
(
entities,
server_event_bus,
time,
player_metrics,
uids,
chat_modes,
players,
clients,
): Self::SystemData,
(entities, server_event_bus, time, uids, chat_modes, players, clients): Self::SystemData,
) {
let mut server_emitter = server_event_bus.emitter();
@ -111,7 +97,6 @@ impl<'a> System<'a> for Sys {
entity,
client,
player,
&player_metrics,
&uids,
&chat_modes,
msg,

View File

@ -1,12 +1,11 @@
use crate::{client::Client, metrics::PlayerMetrics, Settings};
use crate::{client::Client, Settings};
use common::{
event::{EventBus, ServerEvent},
resources::Time,
};
use common_ecs::{Job, Origin, Phase, System};
use common_net::msg::PingMsg;
use specs::{Entities, Join, Read, ReadExpect, ReadStorage};
use std::sync::atomic::Ordering;
use specs::{Entities, Join, Read, ReadStorage};
use tracing::{debug, info};
impl Sys {
@ -28,7 +27,6 @@ impl<'a> System<'a> for Sys {
Entities<'a>,
Read<'a, EventBus<ServerEvent>>,
Read<'a, Time>,
ReadExpect<'a, PlayerMetrics>,
ReadStorage<'a, Client>,
Read<'a, Settings>,
);
@ -39,7 +37,7 @@ impl<'a> System<'a> for Sys {
fn run(
_job: &mut Job<Self>,
(entities, server_event_bus, time, player_metrics, clients, settings): Self::SystemData,
(entities, server_event_bus, time, clients, settings): Self::SystemData,
) {
let mut server_emitter = server_event_bus.emitter();
@ -48,14 +46,11 @@ impl<'a> System<'a> for Sys {
match res {
Err(e) => {
if !client.terminate_msg_recv.load(Ordering::Relaxed) {
debug!(?entity, ?e, "network error with client, disconnecting");
player_metrics
.clients_disconnected
.with_label_values(&["network_error"])
.inc();
server_emitter.emit(ServerEvent::ClientDisconnect(entity));
}
debug!(?entity, ?e, "network error with client, disconnecting");
server_emitter.emit(ServerEvent::ClientDisconnect(
entity,
common::comp::DisconnectReason::NetworkError,
));
},
Ok(1_u64..=u64::MAX) => {
// Update client ping.
@ -66,14 +61,11 @@ impl<'a> System<'a> for Sys {
if time.0 - last_ping > settings.client_timeout.as_secs() as f64
// Timeout
{
if !client.terminate_msg_recv.load(Ordering::Relaxed) {
info!(?entity, "timeout error with client, disconnecting");
player_metrics
.clients_disconnected
.with_label_values(&["timeout"])
.inc();
server_emitter.emit(ServerEvent::ClientDisconnect(entity));
}
info!(?entity, "timeout error with client, disconnecting");
server_emitter.emit(ServerEvent::ClientDisconnect(
entity,
common::comp::DisconnectReason::Timeout,
));
} else if time.0 - last_ping > settings.client_timeout.as_secs() as f64 * 0.5 {
// Try pinging the client if the timeout is nearing.
client.send_fallible(PingMsg::Ping);

View File

@ -145,7 +145,10 @@ impl<'a> System<'a> for Sys {
.find(|(_, _, old_player)| old_player.uuid() == uuid)
{
// Remove old client
server_event_bus.emit_now(ServerEvent::ClientDisconnect(old_entity));
server_event_bus.emit_now(ServerEvent::ClientDisconnect(
old_entity,
common::comp::DisconnectReason::NewerLogin,
));
let _ = old_client.send(ServerGeneral::Disconnect(DisconnectReason::Kicked(
String::from("You have logged in from another location."),
)));