Skip to content
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

Add caching, network handler improvements, and performance improvements #2

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Opalinium
Copy link

Proposed changes reduce API calls by caching a players timezone for a determinate period of time (1 day) and similarly, local-time placeholders are much more consistent with a retryDelay being ran in the event that a players timezone is unable to be fetched due to API rate-limits.

This should functionally mitigate a common occurrence of the expansion being "unable to fetch a players timezone" due to excessive API calls resulting in a rate-limit. The expansion has also been made to be completely thread-safe

Improved Network connection handling via a connection read timeout, with exception handling
Improved API Response Handler - BufferedReader now reads the entire response string and checks whether it is null before assigning it to the 'timezone' variable
When the API call fails, added a new BukkitRunnable that will be scheduled to run after the retryDelay. It will retry adding the timezone to the map after the delay instead of adding it immediately. This way, we prevent overloading the API
getTimeZone now uses a new thread instead of a BukkitRunnable to handle the API request
Integrated 'ConcurrentHashMap' in place of 'Hashmap' to ensure the expansion is thread-safe
retryDelay was also migrated to a new 'getTimeZoneFromAPI' method, and appropriately initialized
I really need to sleep...
Implement cache expiration in DateManager
Optimize cache structure in DateManager
Return CompletableFuture in DateManager's getTimeZone
Adapt LocalTimeExpansion to work with updated DateManager
…e efficiency

Remove cache expiry checks from getTimeZone method
Remove data removal from onLeave event handler
Create a new scheduled task removeExpiredEntries to handle cache expiry and player data removal
Update isCacheExpired method to remove players from the timezones map when cache expires
Add a check in removeExpiredEntries to skip non-UUID keys in the cache
Remove unnecessary onLeave event handler and Listener implementation
`Shutdown` method to shut down the executor service on `clear()`
Fixed `getTimeZone` method to be non-blocking
Removed premature return of `CompletableFuture`
@Opalinium Opalinium requested a review from iGabyTM July 14, 2023 12:55
@Opalinium Opalinium requested a review from iGabyTM July 15, 2023 00:45
@untuned
Copy link

untuned commented Jul 16, 2023

Could support be added for using an API key? People on shared hosts may not be able to use this regardless if other people on the same machine are making API calls. See: #3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants