-
Notifications
You must be signed in to change notification settings - Fork 478
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
feat: Add caching mechanism to optimize weather data fetching from Op… #197
feat: Add caching mechanism to optimize weather data fetching from Op… #197
Conversation
backends/open-meteo.com.go
Outdated
@@ -161,11 +171,22 @@ func parseCurCond(current curCond) (ret iface.Cond) { | |||
|
|||
} | |||
|
|||
var mu sync.Mutex |
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.
Why do you need a mutex? None of the code uses any goroutines or is parallelized from what I know?
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.
Great question! The mutex is there to keep the cache safe if we get multiple requests at the same time. Even though we’re not using goroutines now, it’s a good precaution for the future. I can look into it again if needed!
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.
I'd suggest remove it given its not useful right now. It's "dead code"
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.
I understand your concern about the mutex since we aren't using goroutines in the current implementation. I can remove it to simplify the code. Thanks for the feedback!
backends/open-meteo.com.go
Outdated
@@ -110,6 +118,8 @@ var ( | |||
func (opmeteo *openmeteoConfig) Setup() { | |||
flag.StringVar(&opmeteo.apiKey, "openmeteo-api-key", "", "openmeteo backend: the api `KEY` to use if commercial usage") | |||
flag.BoolVar(&opmeteo.debug, "openmeteo-debug", false, "openmeteo backend: print raw requests and responses") | |||
opmeteo.cache = make(map[string]cachedData) // initializing a cache | |||
opmeteo.cacheDuration = 1 * time.Hour // setting the cache duation to 1 hour |
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.
Should the cache duration be configurable?
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.
Good point! Making the cache duration configurable would give users more flexibility. I can add that feature to allow them to set it according to their needs.
5b6d45c
to
6578d5e
Compare
Issue: #196
Motivation and Context
This PR introduces caching to the weather backend using the OpenMeteo API. It improves performance by storing weather data for each location, reducing the number of API calls and ensuring data freshness for a specific period. It addresses the need to limit repeated API requests within a short time frame, enhancing the application's efficiency.
Key Benefits:
Reduced API request load.
Faster response times for repeated requests.
Improved resource management and performance.
Description
Added caching functionality:
Introduced a cache map to store weather data by location, along with timestamps.
Data remains valid for a specified cacheDuration (set to 1 hour).
Before making API calls, the cache is checked for existing, valid data.
Enhanced the Fetch function:
It now checks the cache before fetching fresh data from the OpenMeteo API.
If the cache contains valid data (based on timestamp), it returns cached data.
Otherwise, it fetches new data, updates the cache, and returns the updated result.
Synchronization using mutex locks:
Added a sync.Mutex to ensure thread-safe access to the cache map, preventing race conditions in concurrent requests.
Steps for Testing
Setup:
Pull the latest changes from this branch.
Ensure the Go environment is properly set up with required dependencies (e.g., github.com/schachmat/wego/iface).
Testing the caching functionality:
Run the backend and make a request to the OpenMeteo API for a specific location.
Check the logs to confirm that a fresh API request is made.
Wait less than 1 hour and make the same request for the same location again. The cache should be used, reducing the API call.
Wait for more than 1 hour, and ensure that a new API request is made after cache expiration.
Check concurrency:
Simulate multiple concurrent requests for the same location to verify that caching works without race conditions or deadlocks.
Screenshots (if applicable)
N/A for this PR (backend changes are not visual).