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

Update InitializeEdgesWorker.java to refresh Edges every 30 minutes #2544

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

Sn0w3y
Copy link
Contributor

@Sn0w3y Sn0w3y commented Feb 15, 2024

Right now if a new Edge Device is added to the Backend we always need to restart the Backend. This fix handles this as it calls the InitializeEdgesWorker every 30 Minutes without setting the Edges Offline again after first run.

Right now if a new Edge Device is added to the Backend we always need to restart the Backend. This fix handles this as it calls the InitializeEdgesWorker every 30 Minutes without setting the Edges Offline again after first run.
Copy link
Contributor

@sfeilmeier sfeilmeier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check if all changes are actually required: https://github.com/OpenEMS/openems/pull/2544/files

All comments must be in English language.

I am not 100 % convinced that this approach is a good idea... but let's discuss it on a "clean as possible" PR.

Copy link

github-actions bot commented Mar 5, 2024

Code Coverage

Changed Comments to English, Changed the Formatting back to the original one
@Sn0w3y
Copy link
Contributor Author

Sn0w3y commented Mar 5, 2024

@sfeilmeier i guess it looks better now :)

Summary of Changes in InitializeEdgesWorker

We've made several key updates to the InitializeEdgesWorker class aimed at enhancing its functionality, efficiency, and reliability. Below is an overview of these changes and the motivations behind them:

Key Updates

  1. Scheduled Task Execution:

    • Previously: The class executed a one-time cache update at startup.
    • Now: We've introduced a ScheduledExecutorService to periodically update the cache every 30 minutes. This ensures the cache remains up-to-date without manual intervention.
  2. Enhanced Error Handling:

    • Improvement: Enhanced logging for error scenarios, particularly when connecting to Postgres, to aid in troubleshooting.
  3. Optimization:

    • Change: Added a flag (isMarkAllEdgesAsOfflineCalled) to ensure that the operation to mark all edges as offline is executed only once. This prevents unnecessary database updates.
  4. Code Organization:

    • Refactoring: Centralized the caching logic within runCachingEdgesTask, simplifying the start() method and reducing code duplication.

Rationale and Specific Issue Addressed

The introduction of periodic caching is not only aimed at keeping the data fresh but also at addressing a specific issue observed in the system. When a new edge device is added to the backend, the UI currently experiences a hang-up, failing to display the new device immediately. This situation results in a less responsive and potentially confusing user experience.

With the newly implemented periodic cache update, we ensure that any new edge devices are integrated into the cache within a maximum of 30 minutes, thereby "fixing" the issue of the UI hang-up by ensuring that new devices are eventually displayed without requiring manual intervention or a system restart.

This approach significantly improves the system's robustness, ensuring that changes in the backend are accurately and timely reflected in the UI, enhancing the overall user experience.

Conclusion

These updates are intended to make the InitializeEdgesWorker more robust, efficient, and easier to maintain. More importantly, they address a specific operational issue that affects user interaction with the system. We believe these changes align with our goals for improving data accuracy, system performance, and user experience. We welcome any feedback or suggestions for further improvements.

Do you maybe think it is better to somehow "subscribe" to the Database and check for changes and only then Update the EdgeCache?

Copy link

github-actions bot commented Mar 5, 2024

Code Coverage

@sfeilmeier
Copy link
Contributor

@michaelgrill: Can you please add your opinion?

@sfeilmeier sfeilmeier requested a review from michaelgrill March 11, 2024 08:33
Copy link

github-actions bot commented Apr 1, 2024

Code Coverage

Copy link
Contributor

@michaelgrill michaelgrill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should work for this case, but the long term goal would be to remove the "cache" completly

Copy link

github-actions bot commented Apr 2, 2024

Code Coverage

@Sn0w3y Sn0w3y requested a review from michaelgrill April 2, 2024 16:17
Copy link

Code Coverage

@Sn0w3y Sn0w3y requested a review from sfeilmeier April 30, 2024 17:18
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