Merge branch 'xvar/persistence-fixes' into 'master'

Fixed a rare server crash when creating a character

See merge request veloren/veloren!2237
This commit is contained in:
Marcel 2021-05-03 18:09:59 +00:00
commit f5c7f99846
6 changed files with 149 additions and 149 deletions

View File

@ -1,8 +1,8 @@
use crate::persistence::character_loader::CharacterLoader;
use crate::persistence::character_updater::CharacterUpdater;
use common::comp::{
inventory::loadout_builder::LoadoutBuilder, Body, Inventory, Item, SkillSet, Stats,
};
use specs::{Entity, ReadExpect};
use specs::{Entity, WriteExpect};
const VALID_STARTER_ITEMS: [&str; 6] = [
"common.items.weapons.hammer.starter_hammer",
@ -19,7 +19,7 @@ pub fn create_character(
character_alias: String,
character_tool: Option<String>,
body: Body,
character_loader: &ReadExpect<'_, CharacterLoader>,
character_updater: &mut WriteExpect<'_, CharacterUpdater>,
) {
// quick fix whitelist validation for now; eventually replace the
// `Option<String>` with an index into a server-provided list of starter
@ -55,7 +55,7 @@ pub fn create_character(
let waypoint = None;
character_loader.create_character(
character_updater.create_character(
entity,
player_uuid,
character_alias,

View File

@ -26,7 +26,7 @@ use crate::{
};
use common::character::{CharacterId, CharacterItem, MAX_CHARACTERS_PER_PLAYER};
use core::ops::Range;
use rusqlite::{types::Value, ToSql, Transaction, NO_PARAMS};
use rusqlite::{types::Value, Connection, ToSql, Transaction, NO_PARAMS};
use std::{collections::VecDeque, rc::Rc};
use tracing::{error, trace, warn};
@ -53,10 +53,7 @@ struct CharacterContainers {
/// BFS the inventory/loadout to ensure that each is topologically sorted in the
/// sense required by convert_inventory_from_database_items to support recursive
/// items
pub fn load_items_bfs(
connection: &mut Transaction,
root: i64,
) -> Result<Vec<Item>, PersistenceError> {
pub fn load_items_bfs(connection: &Connection, root: i64) -> Result<Vec<Item>, PersistenceError> {
let mut items = Vec::new();
let mut queue = VecDeque::new();
queue.push_front(root);
@ -100,7 +97,7 @@ pub fn load_items_bfs(
pub fn load_character_data(
requesting_player_uuid: String,
char_id: CharacterId,
connection: &mut Transaction,
connection: &Connection,
) -> CharacterDataResult {
let character_containers = get_pseudo_containers(connection, char_id)?;
let inventory_items = load_items_bfs(connection, character_containers.inventory_container_id)?;
@ -216,10 +213,7 @@ pub fn load_character_data(
/// In the event that a join fails, for a character (i.e. they lack an entry for
/// stats, body, etc...) the character is skipped, and no entry will be
/// returned.
pub fn load_character_list(
player_uuid_: &str,
connection: &mut Transaction,
) -> CharacterListResult {
pub fn load_character_list(player_uuid_: &str, connection: &Connection) -> CharacterListResult {
let characters;
{
#[rustfmt::skip]
@ -619,7 +613,7 @@ fn get_new_entity_ids(
/// Fetches the pseudo_container IDs for a character
fn get_pseudo_containers(
connection: &mut Transaction,
connection: &Connection,
character_id: CharacterId,
) -> Result<CharacterContainers, PersistenceError> {
let character_containers = CharacterContainers {
@ -639,7 +633,7 @@ fn get_pseudo_containers(
}
fn get_pseudo_container_id(
connection: &mut Transaction,
connection: &Connection,
character_id: CharacterId,
pseudo_container_position: &str,
) -> Result<EntityId, PersistenceError> {

View File

@ -1,13 +1,13 @@
use crate::persistence::{
character::{create_character, delete_character, load_character_data, load_character_list},
character::{load_character_data, load_character_list},
error::PersistenceError,
establish_connection, DatabaseSettings, PersistedComponents,
establish_connection, ConnectionMode, DatabaseSettings, PersistedComponents,
};
use common::character::{CharacterId, CharacterItem};
use crossbeam_channel::{self, TryIter};
use rusqlite::Transaction;
use rusqlite::Connection;
use std::sync::{Arc, RwLock};
use tracing::{error, trace};
use tracing::error;
pub(crate) type CharacterListResult = Result<Vec<CharacterItem>, PersistenceError>;
pub(crate) type CharacterCreationResult =
@ -17,15 +17,6 @@ type CharacterLoaderRequest = (specs::Entity, CharacterLoaderRequestKind);
/// Available database operations when modifying a player's character list
enum CharacterLoaderRequestKind {
CreateCharacter {
player_uuid: String,
character_alias: String,
persisted_components: PersistedComponents,
},
DeleteCharacter {
player_uuid: String,
character_id: CharacterId,
},
LoadCharacterList {
player_uuid: String,
},
@ -89,38 +80,18 @@ impl CharacterLoader {
.spawn(move || {
// Unwrap here is safe as there is no code that can panic when the write lock is
// taken that could cause the RwLock to become poisoned.
let mut conn = establish_connection(&*settings.read().unwrap());
//
// This connection -must- remain read-only to avoid lock contention with the
// CharacterUpdater thread.
let mut conn =
establish_connection(&*settings.read().unwrap(), ConnectionMode::ReadOnly);
for request in internal_rx {
conn.update_log_mode(&settings);
match conn.connection.transaction() {
Ok(mut transaction) => {
let response =
CharacterLoader::process_request(request, &mut transaction);
if !response.is_err() {
match transaction.commit() {
Ok(()) => {
trace!("Commit for character loader completed");
},
Err(e) => error!(
"Failed to commit transaction for character loader, \
error: {:?}",
e
),
};
};
if let Err(e) = internal_tx.send(response) {
error!(?e, "Could not send character loader response");
}
},
Err(e) => {
error!(
"Failed to start transaction for character loader, error: {:?}",
e
)
},
let response = CharacterLoader::process_request(request, &conn);
if let Err(e) = internal_tx.send(response) {
error!(?e, "Could not send character loader response");
}
}
})
@ -137,41 +108,23 @@ impl CharacterLoader {
// CharacterLoaderResponse::is_err()
fn process_request(
request: CharacterLoaderRequest,
mut transaction: &mut Transaction,
connection: &Connection,
) -> CharacterLoaderResponse {
let (entity, kind) = request;
CharacterLoaderResponse {
entity,
result: match kind {
CharacterLoaderRequestKind::CreateCharacter {
player_uuid,
character_alias,
persisted_components,
} => CharacterLoaderResponseKind::CharacterCreation(create_character(
&player_uuid,
&character_alias,
persisted_components,
&mut transaction,
)),
CharacterLoaderRequestKind::DeleteCharacter {
player_uuid,
character_id,
} => CharacterLoaderResponseKind::CharacterList(delete_character(
&player_uuid,
character_id,
&mut transaction,
)),
CharacterLoaderRequestKind::LoadCharacterList { player_uuid } => {
CharacterLoaderResponseKind::CharacterList(load_character_list(
&player_uuid,
&mut transaction,
connection,
))
},
CharacterLoaderRequestKind::LoadCharacterData {
player_uuid,
character_id,
} => {
let result = load_character_data(player_uuid, character_id, &mut transaction);
let result = load_character_data(player_uuid, character_id, connection);
if result.is_err() {
error!(
?result,
@ -184,45 +137,6 @@ impl CharacterLoader {
}
}
/// Create a new character belonging to the player identified by
/// `player_uuid`
pub fn create_character(
&self,
entity: specs::Entity,
player_uuid: String,
character_alias: String,
persisted_components: PersistedComponents,
) {
if let Err(e) = self
.update_tx
.send((entity, CharacterLoaderRequestKind::CreateCharacter {
player_uuid,
character_alias,
persisted_components,
}))
{
error!(?e, "Could not send character creation request");
}
}
/// Delete a character by `id` and `player_uuid`
pub fn delete_character(
&self,
entity: specs::Entity,
player_uuid: String,
character_id: CharacterId,
) {
if let Err(e) = self
.update_tx
.send((entity, CharacterLoaderRequestKind::DeleteCharacter {
player_uuid,
character_id,
}))
{
error!(?e, "Could not send character deletion request");
}
}
/// Loads a list of characters belonging to the player identified by
/// `player_uuid`
pub fn load_character_list(&self, entity: specs::Entity, player_uuid: String) {

View File

@ -4,7 +4,7 @@ use common::character::CharacterId;
use crate::persistence::{
character_loader::{CharacterLoaderResponse, CharacterLoaderResponseKind},
error::PersistenceError,
establish_connection, DatabaseSettings, VelorenConnection,
establish_connection, ConnectionMode, DatabaseSettings, PersistedComponents, VelorenConnection,
};
use crossbeam_channel::TryIter;
use rusqlite::DropBehavior;
@ -20,8 +20,15 @@ use tracing::{debug, error, info, trace, warn};
pub type CharacterUpdateData = (comp::SkillSet, comp::Inventory, Option<comp::Waypoint>);
#[allow(clippy::large_enum_variant)]
pub enum CharacterUpdaterEvent {
BatchUpdate(Vec<(CharacterId, CharacterUpdateData)>),
CreateCharacter {
entity: Entity,
player_uuid: String,
character_alias: String,
persisted_components: PersistedComponents,
},
DeleteCharacter {
entity: Entity,
requesting_player_uuid: String,
@ -58,7 +65,8 @@ impl CharacterUpdater {
.spawn(move || {
// Unwrap here is safe as there is no code that can panic when the write lock is
// taken that could cause the RwLock to become poisoned.
let mut conn = establish_connection(&*settings.read().unwrap());
let mut conn =
establish_connection(&*settings.read().unwrap(), ConnectionMode::ReadWrite);
while let Ok(updates) = update_rx.recv() {
match updates {
CharacterUpdaterEvent::BatchUpdate(updates) => {
@ -80,6 +88,35 @@ impl CharacterUpdater {
.store(true, Ordering::Relaxed);
};
},
CharacterUpdaterEvent::CreateCharacter {
entity,
character_alias,
player_uuid,
persisted_components,
} => {
match execute_character_create(
entity,
character_alias,
&player_uuid,
persisted_components,
&mut conn,
) {
Ok(response) => {
if let Err(e) = response_tx.send(response) {
error!(?e, "Could not send character creation response");
} else {
debug!(
"Processed character create for player {}",
player_uuid
);
}
},
Err(e) => error!(
"Error creating character for player {}, error: {:?}",
player_uuid, e
),
}
},
CharacterUpdaterEvent::DeleteCharacter {
entity,
requesting_player_uuid,
@ -165,6 +202,28 @@ impl CharacterUpdater {
.load(Ordering::Relaxed)
}
pub fn create_character(
&mut self,
entity: Entity,
requesting_player_uuid: String,
alias: String,
persisted_components: PersistedComponents,
) {
if let Err(e) =
self.update_tx
.as_ref()
.unwrap()
.send(CharacterUpdaterEvent::CreateCharacter {
entity,
player_uuid: requesting_player_uuid,
character_alias: alias,
persisted_components,
})
{
error!(?e, "Could not send character creation request");
}
}
pub fn delete_character(
&mut self,
entity: Entity,
@ -223,22 +282,6 @@ impl CharacterUpdater {
}
}
/// Updates a single character based on their id and components
pub fn update(
&mut self,
character_id: CharacterId,
skill_set: &comp::SkillSet,
inventory: &comp::Inventory,
waypoint: Option<&comp::Waypoint>,
) {
self.batch_update(std::iter::once((
character_id,
skill_set,
inventory,
waypoint,
)));
}
/// Indicates to the batch update thread that a requested disconnection of
/// all clients has been processed
pub fn disconnected_success(&mut self) {
@ -274,6 +317,32 @@ fn execute_batch_update(
Ok(())
}
fn execute_character_create(
entity: Entity,
alias: String,
requesting_player_uuid: &str,
persisted_components: PersistedComponents,
connection: &mut VelorenConnection,
) -> Result<CharacterLoaderResponse, PersistenceError> {
let mut transaction = connection.connection.transaction()?;
let response = CharacterLoaderResponse {
entity,
result: CharacterLoaderResponseKind::CharacterCreation(super::character::create_character(
requesting_player_uuid,
&alias,
persisted_components,
&mut transaction,
)),
};
if !response.is_err() {
transaction.commit()?;
};
Ok(response)
}
fn execute_character_delete(
entity: Entity,
requesting_player_uuid: &str,

View File

@ -13,6 +13,7 @@ use refinery::Report;
use rusqlite::{Connection, OpenFlags};
use std::{
fs,
ops::Deref,
path::PathBuf,
sync::{Arc, RwLock},
time::Duration,
@ -69,6 +70,12 @@ impl VelorenConnection {
}
}
impl Deref for VelorenConnection {
type Target = Connection;
fn deref(&self) -> &Connection { &self.connection }
}
fn set_log_mode(connection: &mut Connection, sql_log_mode: SqlLogMode) {
// Rusqlite's trace and profile logging are mutually exclusive and cannot be
// used together
@ -92,6 +99,12 @@ pub struct DatabaseSettings {
pub sql_log_mode: SqlLogMode,
}
#[derive(Clone, Copy, PartialEq)]
pub enum ConnectionMode {
ReadOnly,
ReadWrite,
}
#[derive(Clone, Copy, Debug, PartialEq)]
pub enum SqlLogMode {
/// Logging is disabled
@ -104,7 +117,7 @@ pub enum SqlLogMode {
/// Runs any pending database migrations. This is executed during server startup
pub fn run_migrations(settings: &DatabaseSettings) {
let mut conn = establish_connection(settings);
let mut conn = establish_connection(settings, ConnectionMode::ReadWrite);
diesel_to_rusqlite::migrate_from_diesel(&mut conn)
.expect("One-time migration from Diesel to Refinery failed");
@ -131,22 +144,32 @@ fn rusqlite_profile_callback(log_message: &str, dur: Duration) {
info!("{} Duration: {:?}", log_message, dur);
}
pub(crate) fn establish_connection(settings: &DatabaseSettings) -> VelorenConnection {
pub(crate) fn establish_connection(
settings: &DatabaseSettings,
connection_mode: ConnectionMode,
) -> VelorenConnection {
fs::create_dir_all(&settings.db_dir).expect(&*format!(
"Failed to create saves directory: {:?}",
&settings.db_dir
));
let connection = Connection::open_with_flags(
&settings.db_dir.join("db.sqlite"),
OpenFlags::SQLITE_OPEN_PRIVATE_CACHE | OpenFlags::default(),
)
.unwrap_or_else(|err| {
panic!(
"Error connecting to {}, Error: {:?}",
settings.db_dir.join("db.sqlite").display(),
err
)
});
let open_flags = OpenFlags::SQLITE_OPEN_PRIVATE_CACHE
| OpenFlags::SQLITE_OPEN_NO_MUTEX
| match connection_mode {
ConnectionMode::ReadWrite => {
OpenFlags::SQLITE_OPEN_CREATE | OpenFlags::SQLITE_OPEN_READ_WRITE
},
ConnectionMode::ReadOnly => OpenFlags::SQLITE_OPEN_READ_ONLY,
};
let connection = Connection::open_with_flags(&settings.db_dir.join("db.sqlite"), open_flags)
.unwrap_or_else(|err| {
panic!(
"Error connecting to {}, Error: {:?}",
settings.db_dir.join("db.sqlite").display(),
err
)
});
let mut veloren_connection = VelorenConnection::new(connection);

View File

@ -126,7 +126,7 @@ impl Sys {
alias,
tool,
body,
character_loader,
character_updater,
);
}
},