Make DB access async, add nanotime to debug, move mark to NameRecord. #16
Conversation
|
Can one of the admins verify this patch? @ProgrammerDan @Maxopoly "ok to test" will build any commits made to this PR |
|
Player uuid was recorded to have an administrative record of who created a
wordbank. Used it to figure out how many users were leveraging the minigame
upon release. Clearly I never got around to making it easily digested for
non db loving admins :) oops!
Neat PR, first glance looks decent. What's the level of testing so far?
…On Fri, May 8, 2020, 07:09 Bot Bottinson ***@***.***> wrote:
Can one of the admins verify this patch? @ProgrammerDan
<https://github.com/ProgrammerDan> @Maxopoly <https://github.com/Maxopoly>
"ok to test" will build any commits made to this PR
"test this please" will build this PR once
"retest this please" will make another single build for this PR
"add to whitelist" will whitelist the creator of this PR and make jenkins
automatically build any PRs made by him
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#16 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADD5DT3VM2QB625MSQNC3DRQPR5HANCNFSM4M4C3FPA>
.
|
|
Build finished. |
|
Bug testing - next to none. Feature testing - Of note is that due to the name lookup/generate only being triggered by the "first hit" and the map entry marking a player's first hit only being removed after either the 10 second task timer runs out or the player successfully applyies a cached name to an item, a player cannot (usually) cause more than one lookup/generate to run at a time, ie. "spamming wordbank to lag the server/database" was unintentionally (somewhat) nerfed. |
|
Build finished. |
With the async code put in place of the old non-async code, the player would be charged the cost of a wordbank item even if the new name hadn't finished loading yet. That's been fixed. The unintentional cooldown on wordbank usage while a player waits for a name to load, except sometimes not if player switches items, that's been made more predictable now: if a player queues a name then tries to queue another name without applying the first name, it'll warn them first and either prevent them from loading the new name before the last name loads, or allow them to queue the next name load regardless of whether the last finished loading or not. Configurable with "prevent_dblookup_spam", defaults to true, preventing players from queueing second+ name lookups until the first has finished or the hit-to-confirm timer ends. The config option added in the last commit "force_mark_all_renames" has been renamed to "dblog_all_item_marks" following crimeo's (very) valid criticism of how awful and confusing the old name was. The in-game error messages were also changed to red to be more noticeable.
|
Build finished. |
|
How're you feeling about this commit? Feeling it's complete? or still planning more work? |
|
Build finished. |
|
Yeah I'm not adding anything new, and I can't find anything else to break with these changes. |
|
Ok, thanks, will review & merge asap |
Used bukkit scheduler + guava loadingcache to make name loading/generation/storage async
Added 3 config options (only present in code, not config.yml) to configure the cache
Added time measuring debug to the name loading/generation/storage
I wasn't sure whether the player ID/item type was being written to the database for logging purposes so "mark"ing a name as used (and including the name/item) is still done for every wordbanked item. That can be safely disabled by calling NameRecord.mark with force=false on line 183 of ActionListener.