game_stats.startTime is currently set implicitly by DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP which, if you think about it for one second, is not a smart thing to do.

This causes any game that have any of its values updated (like validity) when the game ends to have its startTime set to the game end time. Outch!

Hi,

I have free time and desire to improve Docker related stuff here.
How can I contribute, is it needed anyways?

PS: This will be a practice for me and I want to help the FAF community. If you would like to know who I am and what I do, please send me a private message (I do not know if it's possible in github)

If we could unify all of our database fixtures into the DB repo that'd be nice, no rush on this, of course.

The server has been storing player stats for desynced games.

We should run a script that removes such invalid stats, as suggested by ChrisKitching:
https://github.com/FAForever/server/pull/178/files#diff-9e7e868d3d7a27585abd7a3be83deb85R275

If it's worth doing that? Not sure where player stats are used and if retroactively cleaning them up would change anything

game_player_stats.playerId references login but has no constraint set; there are players referenced who do no longer exist in login.

Proposed fix:

ALTER TABLE game_player_stats ADD CONSTRAINT fk_game_player_stats_player FOREIGN KEY (playerId) REFERENCES login(id);

Having a view makes it much simpler to access it by different applications

• Make sure Server & API are stopped
• Back up database:

docker exec -i stable_faf-db_1 mysqldump --login-path=faf_lobby --databases faf_lobby > /opt/stable/db_dumps/faf_lobby-$(date +"%Y-%m-%d-%H-%M-%S").sql

• Since we never properly upgraded to mysql 5.7, execute:

docker exec -ti stable_faf-db_1 mysql_upgrade -u root -p

• Since our passwords for root@localhost and [email protected] are different, make them equal:

docker exec -i stable_faf-db_1 mysql --login-path=faf_lobby -e "alter user 'root'@'127.0.0.1' identified by 'ENTERPASSWORDHERE';"

• Merge origin/develop into origin/master
• Tag origin/master as v5
• Create & adjust flyway.conf (change DB and password):

cd /opt/stable/db_repo/
git fetch && git checkout v5
cp example_flyway.conf flyway.conf
vi flyway.conf

• Install flyway into DB container by either rebuilding it or executing:

docker exec -i stable_faf-db_1 bash -c 'apt-get update \
    && apt-get install wget -y \
    && wget -q http://repo1.maven.org/maven2/org/flywaydb/flyway-commandline/4.0.3/flyway-commandline-4.0.3-linux-x64.tar.gz \
    && tar xzf flyway-commandline-4.0.3-linux-x64.tar.gz \
    && rm flyway-commandline-4.0.3-linux-x64.tar.gz \
    && mv flyway-4.0.3 flyway \
    && chmod +x flyway/flyway \
    && wget -q http://search.maven.org/remotecontent?filepath=mysql/mysql-connector-java/6.0.4/mysql-connector-java-6.0.4.jar -O flyway/drivers/mysql-connector-java-6.0.4.jar'

• Start migration

./migrate.sh

Name of docker container: stable_faf-db_1
Is this the first time Flyway runs on this database? y
Does the database already contain data? y

Migration is expected to run successfully:

Database: jdbc:mysql://127.0.0.1:3306/faf_lobby?useSSL=false (MySQL 5.7)
Creating Metadata table: `faf_lobby`.`schema_version`
Successfully baselined schema with version: 1
Flyway 4.0.3 by Boxfuse

Database: jdbc:mysql://127.0.0.1:3306/faf_lobby?useSSL=false (MySQL 5.7)
Successfully validated 5 migrations (execution time 00:00.023s)
Current version of schema `faf_lobby`: 1
Migrating schema `faf_lobby` to version 2 - add-clan-tag-colors
Migrating schema `faf_lobby` to version 3 - achievement-create-time
Migrating schema `faf_lobby` to version 4 - achievements-foreign-keys
Migrating schema `faf_lobby` to version 5 - mod migration
Successfully applied 4 migrations to schema `faf_lobby` (execution time 00:29.766s).

Fallback

If shit hits the fan, restore the old Database:

docker exec -i stable_faf-db_1 mysql --login-path=faf_lobby -e "drop database faf_lobby;"
docker exec -i stable_faf-db_1 mysql --login-path=faf_lobby < /opt/stable/db_dumps/faf_lobby-DATETIME.sql

The table "teamkills" does not have a primary key.
And its two fields victim and teamkiller don't aren't specified as foreign keys.

Such as:
https://github.com/FAForever/db/blob/develop/db-structure.sql#L25

score is currently non-null and initialized with -1 by the server which basically means "no score".
Instead, score should be nullable. Since the server sets it explicitly anyway and is the only software that sets it, we don't need to fix anything on the server.

The coop_leaderboard currently doesn't store the number of players that were in the game, requiring an extensive query to get this information.
One might argue that this is redundant data; however, time is stored, being just as redundant as the player count.

While the extensive query isn't very expensive, things are much easier without (especially to understand). So I'm going to add player_count as a field.

To make it easier for beginners we should provide a container with dump data at https://hub.docker.com.

We have a lot of game_stats.mapId with value 0. I suspect a casing problem in the respective query on the server (filename upper/lowercase) which needs to be fixed.

After this is sorted out, we should restore as many map IDs as we can by reading the maps from the replay files.

Last step is to decide/change whether mapId should be nullable or not; currently it's not, but players can play maps that aren't in our database (think of map creators). The question is if we want to store such games at all; if yes, then mapId needs to be nullable.

We have games that have been played with mods that are no longer in our database.
This should never ever happen again.

Requirement from the Mod App Specification

Permission System
Not every mod can use every feature

I suggest to introduce a classic permission system:

• You ask for a special permission, e.g. ban.create, ban.delete
• Admins can define roles with a set of permissions, e.g.role mod with permission ban.*, avatar.*
• Admins can assign permissions or roles to a user

Step 1

• We introduce Permissions

Step 2

• We introduce Roles

FK is there but has no constraint.

If we have OAuth and the new Api, we should start to salt the Login Table:
https://www.wikiwand.com/en/Salt_(cryptography)

Many components depend on this so this module should also build in travis ...

Generate "Joined Community" Date Data

It would be nice to have a simple set of

• a row in oauth_clients for a sample Postman client
• a user anybody can run Postman with

game_stats.host references login but has no constraint set; there are players referenced who do no longer exist in login.

Proposed fix:

INSERT INTO login (id, login, email, password) SELECT
                                                 host,
                                                 concat("Unknown_", host),
                                                 concat("user", host, "@example.com"),
                                                 ""
                                               FROM game_stats
                                               WHERE host NOT IN (SELECT id
                                                                  FROM login)
                                               GROUP BY host;

ALTER TABLE game_stats ADD CONSTRAINT fk_game_stats_host FOREIGN KEY (host) REFERENCES login(id);

In order to be searchable quickly, game_stats and game_player_stats need indices on some fields.

https://github.com/FAForever/api/pull/56/files#diff-5a57b547cd9484b254a2cdafc90d3f02R36

teamkills.id is not auto_increment as it was supposed to be.

db-structure.sql, the result of the migration scripts and the production DB are out of sync.

game_stats.endTime has been removed because, I guess, it was seen as redundant information which can be derived from game_player_stats.scoreTime.

This not only introduces a performance nightmare when querying anything related to endTime - like in the replay vault search - but also isn't even technically correct.

A game does not (have to) end when the last player's score has been reported. In some games, there isn't even a score report because the players just disconnect - so endTime stays empty.

Therefore, the game_stats.endTime need to be brought back and set explicitly by the server.

Sounds easy, but actually isn't:

mysql> select count(*) from clan_members where player_id not in (select id from login);
+----------+
| count(*) |
+----------+
|     1479 |
+----------+

Something is awefully wrong.

There are a few tables that are no longer used (or have never been used) and should therefore be removed:

• auth_group (never used)
• auth_group_permissions (never used)
• auth_permission (never used)
• auth_user (never used)
• avatars_list_copy_812015 (unused, empty)
• bet (no longer used, data has no value for us. Some people were able to "cheat".)
• db_version (unused, was part of the old migration script)
• game_min_rating (unused, redundant)
• game_replays (only used by non-working, unused command command_list in secondaryServer)
• game_replays_old (unused, empty)
• ladder_season_3_safe (unused, empty)
• ladder_divisions (unused, useless data, superseded by ladder_division)
• login_with_duplicated_users (unused, empty)
• replay_vault (unused, superseded by game_stats
• smurf_table (unused, obsolete)
• table_map_broken (unused, superseded by "hidden" flag in new table structure)
• test_game_replays (unused)
• tmp (unused)
• updates (obsolete, unused)
• vault_admin (unused, obsolete data (2 entries))
• view_global_rating (unused, isn't even a view)

They currently don't have a create_time but it would be good to know.

db-structure.sql can be automatic generated in the builld process ...

IMO this project needs the following improvements:

• Initialize the database automatically during container creation if the database does not yet exist instead of having the user execute a setup script manually. Also it should be "production ready", meaning that it assumes to be a productive DB and therefore does not load any test data into the database.
• In order to integrate well with faf-stack, this project should also create databases and users for other tools like IRC, Mediawiki and so on since it's the only project that should have DB root permissions.
• Since faf-server (and potentially faf-api or others) doesn't work properly without some standard "featured" mods in the database, such core data should be added during the DB initialization (it could be added by faf-server if it's missing, but that's not smart since other projects may rely on it as well)
• The Dockerfile should specify a healthcheck (new Docker feature)
• The migration script should be part of the container and executed within the container, not from outside

Currently every AvatarAssignment (table avatars) has a property selected.
Potentially a user can select multiple avatars.

Avatar selection is currently handled by the server ...
https://github.com/FAForever/server/blob/f5c056263fddde5edc4c7927abed61d0fd91e718/server/lobbyconnection.py#L853

1. Avatar Handling should be moved to the API
2. The database model should be adapted, e.g. as an column of login or 1:1 table avatars:login

Hi,

First question - Should the migrations be ran after importing the initial structure into the database?

Second Question - Is there anyway we can get more data for the database tables? For example, it would be useful to have some of the rankings and 1v1 data. I am sure Python has a Faker library to create data for the database, but that would take more time then a simple export.

There are no constraints on the teamkiller and victim columns.

Expected behaviour
https://bugs.mysql.com/bug.php?id=72628

mysql> create table t1(c1 timestamp not null default '2000-01-01 00:00:00');
Query OK, 0 rows affected (0.02 sec)

mysql> insert into t1 values();
Query OK, 1 row affected (0.00 sec)

mysql> insert into t1 values(null);
Query OK, 1 row affected (0.01 sec)

mysql> insert into t1 values(default);
Query OK, 1 row affected (0.00 sec)

mysql> select * from t1;
+---------------------+
| c1                  |
+---------------------+
| 2000-01-01 00:00:00 |
| 2014-08-01 06:11:50 |
| 2000-01-01 00:00:00 |
+---------------------+
3 rows in set (0.00 sec)

Behaviour with 5.7.16

mysql> create table t1(c1 timestamp not null default '2000-01-01 00:00:00');
Query OK, 0 rows affected (0.22 sec)

mysql> insert into t1 values(null);
ERROR 1048 (23000): Column 'c1' cannot be null

• Make sure Server & API are stopped
• Back up database:

cd /opt/stable
tar cvpjf db-backup-$(date +"%Y-%m-%d-%H-%M-%S").tar.bz2 db

• Merge origin/develop into origin/master
• Update DB on prod (get root password first):

cd /opt/stable/db_repo/
git pull
./migrate.sh

Name of docker container: stable_faf-db_1
Database to migrate: faf_lobby
Current database revision: ab3d56fe949550d2ac113a148582e4c7c690e25c

Migration is expected to run successfully:

<noise>
Database is now at revision 5ab8cba965337aa8ec3dc4b5089514e44f834862

The table table_map_features, like many other tables, is a mess that should be cleaned up.

Since it is used by the legacy map vault, we can't just fix it. Therefore, I recommend that we keep the current table as-is and create new ones and views next to it, until we can remove it completely.

Whoever made this should burn in hell.

For some reason unknown to me, parts of game_stats and game_player_stats have been moved to game_stats_bak respective game_player_stats_bak. It's time to merge the table back in and stop having to maintain redundant tables that serve no purpose.
There is no relevant performance impact.

• Stop everything that writes into the database (server, api)
• Backup

cd /opt/stable
docker exec -i stable_faf-db_1 mysqldump --login-path=faf_lobby faf_lobby | gzip -c > db-dump-$(date +"%Y-%m-%d-%H-%M-%S").sql.gzip

• Make sure docker-compose.yml uses build: db_repo for faf-db
• Configure Flyway (because the DB is still called faf_lobby and not faf). In /opt/stable/db_repo/migrate.sh:

flyway.url=jdbc:mysql://127.0.0.1/faf_lobby?localSocket=/var/run/mysqld/mysqld.sock

• Rebuild container (because we updated MySQL)

docker-compose up -d --build faf-db

• Fix the migration version mismatch we have for some reason (doesn't cause harm):

update schema_version set checksum = -427051799 where version = 18;

• Migrate DB

cd /opt/stable/db_repo
git fetch
git checkout v34
docker exec -i stable_faf-db_1 ./migrate.sh

We have no clean way to map the local map to a map on the server.
because the user can e.g. modify the map locally, the map is not uploaded ...

I suggest to add a checksum column to the map_version table.
Then we can identify local broken maps and also implement a auto update map feature ;)

Please vote with 👍 or 👎

For reviewing maps/mods/replays

We should add tests for the migration scripts,
currently the migration is running on an empty database.