-
Notifications
You must be signed in to change notification settings - Fork 417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature] Initial pass at removing qs logging in view of the PlayerEventLogging System #4542
base: master
Are you sure you want to change the base?
Changes from 1 commit
ece6eee
36b499c
f710ada
04298e3
d74a4bd
130bf2f
2b05a12
e3c99a1
436f280
11a2c56
7efcaa2
dd5093f
534a9a0
3f4755e
57825ed
134e408
d6c1022
75f89fe
ab82afa
cfc777b
6605809
7ca379d
8f69f73
8e4aaee
ab7f8d5
78a538e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,17 @@ class PlayerEventSpeechRepository: public BasePlayerEventSpeechRepository { | |
*/ | ||
|
||
// Custom extended repository methods here | ||
static int64 GetNextAutoIncrementId(Database& db) | ||
{ | ||
auto results = db.QueryDatabase( | ||
fmt::format( | ||
"SELECT AUTO_INCREMENT FROM information_schema.tables WHERE TABLE_NAME = '{}';", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should assume that the server user can't directly query There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I ran into a couple of test cases that failed. If I empty an etl table, the max + 1 will return 1, when in fact the auto_increment may be 100. Further, once the retention period is hit, the next id via max + 1 will again be different from the auto_increment. |
||
TableName() | ||
) | ||
); | ||
|
||
return (results.Success() && results.begin()[0] ? strtoll(results.begin()[0], nullptr, 10) : 0); | ||
} | ||
}; | ||
|
||
#endif //EQEMU_PLAYER_EVENT_SPEECH_REPOSITORY_H |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at all of this in ProcessBatchQueue and in our deletions, I think we should wrap these functions in thread calls so we don't tie up the main thread.
This also means that we should create a separate database connection just for processing player events because what happens is even if we had all of this in another thread its going to be lock contended with database locks on the main thread.
World needs to be kept as lightweight as possible in the main thread and this certainly adds weight if anything starts to take more than 500ms which it most certainly could here.
Same would go for using QS if folks use QS to process.
I may end up taking this one on.