feat(server): live database updates #460
Conversation
Would be nice, tested it myself on a one person server, so maybe not realistic |
|
I don’t have an active qbox live server. I think @solareon might though |
Yeah I got one of those. I'm in the middle of the Atlantic Ocean for a few weeks so won't be able to test for a bit. |
| self.PlayerData[key] = val | ||
| self.Functions.Save() |
There was a problem hiding this comment.
If SetPlayerData saves, then why are we calling storage functions before calling SetPlayerData()?
There was a problem hiding this comment.
The save function of the player does not save to the player_groups table, therefore we need to save those separately with storage functions
There was a problem hiding this comment.
The save function is really only to update the players table
There was a problem hiding this comment.
One improvement here would be only writing to the column being changed. That is, introducing additional storage functions, or maybe even exposing a general one. That way we avoid needlessly writing the entire player row when just updating a gang for example. In implementation this would be avoiding calling SetPlayerData, or alternatively within SetPlayerData avoiding calling Save if there is a more targeted option available. Maybe this wouldn't work for all possible keys?
There was a problem hiding this comment.
Overall looks good. I do think we should test this to be sure there isn't a performance regression. Maybe the best way to do so is to have a few people on a test server do a before/after test performing actions like eating food, changing jobs, etc while taking metrics on the database side, server hardware performance, and FiveM server profiler.
| self.PlayerData[key] = val | ||
| self.Functions.Save() |
There was a problem hiding this comment.
One improvement here would be only writing to the column being changed. That is, introducing additional storage functions, or maybe even exposing a general one. That way we avoid needlessly writing the entire player row when just updating a gang for example. In implementation this would be avoiding calling SetPlayerData, or alternatively within SetPlayerData avoiding calling Save if there is a more targeted option available. Maybe this wouldn't work for all possible keys?
| @@ -230,8 +230,8 @@ end | |||
| ---@param tableName string | |||
| ---@return boolean | |||
| local function doesTableExist(tableName) | |||
| local tbl = MySQL.single.await(('SELECT COUNT(*) FROM information_schema.TABLES WHERE TABLE_NAME = \'%s\' AND TABLE_SCHEMA in (SELECT DATABASE())'):format(tableName)) | |||
| return tbl['COUNT(*)'] > 0 | |||
| local result = MySQL.single.await(('SELECT COUNT(*) as count FROM information_schema.TABLES WHERE TABLE_NAME = \'%s\' AND TABLE_SCHEMA in (SELECT DATABASE())'):format(tableName)) | |||
There was a problem hiding this comment.
I think the best thing here might be to select scalar 1 then return not not result. Also, we should use placeholders '?' rather than string format
| ---@return boolean success if operation is successful. | ||
| local function setPlayerPrimaryJob(citizenid, job) | ||
| local affectedRows = MySQL.update.await('UPDATE players SET job = ? WHERE citizenid = ?', {json.encode(job), citizenid}) | ||
| return affectedRows > 0 |
There was a problem hiding this comment.
should we be asserting that the operation is successful, or do you think it's better for it to fail silently?
|
Just out of curiosity, why is this preferred over the current solution, updating the data to the database in bulk on a timer? |
When we bulk write we write all columns, even if only changing one columns value. This PR should improve database performance by making writes smaller. If we find that bulk writes are preferred then we should implement that in the storage layer using a buffer which lets us control the batch size to find the ideal numbers. |
|
I think something that would go along well with a database update like this would be converting the charinfo and metadata columns into their own tables, and using a key/value system with a citizenid as a foreign key so we can get rid of those monsterous json tables. It would be quite easy as well and wouldn't ruin any compatibility, would be very similar to the player groups update. |
Description
Adds small live database updating as opposed to big periodic updates.
Checklist