SD EPG fixes [ci release] #493

jusjoken · 2025-03-08T00:26:08Z

Various commits to fix the SD EPG issues

google-cla · 2025-03-08T00:26:12Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Narflex

Overall good, please address the comments and explain the maintenance issue in the EPG class. (note to self, I reviewed everything completely except EPG.java)

java/sage/epg/sd/SDUtils.java

Narflex · 2025-03-11T21:05:47Z

java/sage/SageConstants.java

Why are you deleting this file?

Sorry. This must have occurred by mistake during my squashing as their was a conflict. I will add it back in in another commit.

java/sage/EPG.java

Narflex · 2025-03-11T21:11:44Z

java/sage/EPG.java


-        if (Sage.time() - wiz.getLastMaintenance() > MAINTENANCE_FREQ)
-          reqMaintenanceType = MaintenanceType.FULL;
+        if(Sage.getBoolean("wizard/scheduled_maintenance", false)){


It looks like you're disabling Wizard maintenance here unless this new property is set (which it will not be by default of course). That's not good, as that will lead to memory bloat if the DB doesn't run it's regular maintenance routine (and users will run out of memory sooner than later without DB maintenance). You may be misunderstanding what 'maintenance' is here...and it's when the Wizard runs through the database and clears out the old unneeded data...it's orthogonal to EPG updates.

Not my intention here. This is still running maintenance but at a specific hour every day. If this property is set to true it still sets maintenance to FULL but the timing is based on the scheduled_maintenance_offset (hour of day) having passed. Hope that makes sense.

OK, now I see...I missed the 'else' on this conditional below before. :) However, to avoid confusion...can you rename the 'EPG update offset' to be something like that...such as wizard/scheduled_epg_update? Maintenance in SageTV does not refer to EPG updates...it refers to when the DB is analyzed and all expired records are removed to reduce memory usage.

Done. I will also mention the change to andy as he uses these properties in his STV change

BTW: Will this new epg scheduled update control affect all types of epg updates, or only Schedules Direct?

It will affect all types of EPG updates...but only if the user has enabled those options.

java/sage/EPG.java

Narflex · 2025-03-11T21:24:41Z

That "approve changes" you see was just for a single commit (and maybe more after that)...I forgot the review applies to the whole PR. :)

Narflex

Fully approve "SD EPG Updates - added seeker/duration_for_watchdog to allow users to…"

Narflex

Fully approve: "Github actions - changed to 20.04 as 18.04 is no longer supported"

Narflex

The commit for "Revert "Add properties to allow changing locator server"" shouldn't be in here...you likely need to do a local 'git pull' from the sagetv repo and then rebase your local branch on top of that...then it should remove that commit from your history.

build.gradle

Narflex

You should squash this commit w/ the other one for the changes to build.gradle, then at least the Linux versioning portion will be more clear.

Narflex

"Release for version 9.2.9 [ci release]" is approved

… extend this process wait if causing issues with large libraries SD EPG Updates - seeker/duration_for_watchdog - fixed debug message to include valid duration

…u 22.04

Narflex · 2025-03-12T22:25:25Z

java/sage/EPG.java


-        if (Sage.time() - wiz.getLastMaintenance() > MAINTENANCE_FREQ)
-          reqMaintenanceType = MaintenanceType.FULL;
+        if(Sage.getBoolean("wizard/scheduled_maintenance", false)){


OK, now I see...I missed the 'else' on this conditional below before. :) However, to avoid confusion...can you rename the 'EPG update offset' to be something like that...such as wizard/scheduled_epg_update? Maintenance in SageTV does not refer to EPG updates...it refers to when the DB is analyzed and all expired records are removed to reduce memory usage.

Narflex · 2025-03-12T22:25:28Z

java/sage/EPG.java

+            if ((Sage.time() - wiz.getLastMaintenance() > MAINTENANCE_FREQ) || ((nextScheuldedMaintenanceTime - MAINTENANCE_FREQ) < Sage.time())){
+                if (Sage.DBG) System.out.println("EPG next scheduled update is ready to run as we are past the maintenance window and/or scheduled time");
+                reqMaintenanceType = MaintenanceType.FULL;
+            }else if(wiz.getLastMaintenance()==0){


I don't think you need this 'else if' block here, because of wiz.getLastMaintenance() is zero...then the first part of the 'if' would be triggered...so this block will never be executed.

Agreed. Removed (I believe I added while testing so I could force an update on restart by setting it to 0 but should have removed this).

Narflex · 2025-03-12T22:27:38Z

java/sage/EPG.java

+        if(Sage.getBoolean("wizard/scheduled_maintenance", false)){
+            if (Sage.DBG) System.out.println("EPG next scheduled maintenance offset is " + Sage.getInt("wizard/" + SCHEDULED_MAINTENANCE_OFFSET, 0));
+
+            nextScheuldedMaintenanceTime = getNextScheduledMaintenanceTime();


nextScheuldedMaintenanceTime is misspelled

Narflex · 2025-03-12T22:30:09Z

java/sage/EPG.java

+                //determine the wait until the next scheduled update time
+                //calc the next time as the user may have changed the schedule settings
+                nextScheuldedMaintenanceTime = getNextScheduledMaintenanceTime();
+                minWait = nextScheuldedMaintenanceTime - Sage.time();


Isn't there a problem where if you set minWait in this block...then it can get overridden on line 687 with the one from the EPGDataSource? So the scheduled maintenance only takes effect if the last EPGDataSource has an update time further out then the scheduled time. It seems like this check should be outside of this loop to override the other update timing logic.

Narflex · 2025-03-12T22:36:17Z

build.gradle

 sageJar.dependsOn cleanSageJar, classes, test
 miniclientJar.dependsOn clean, miniclientClasses, cleanMiniJar
 linuxMiniClientRelease.dependsOn miniclientJar
-sageJar.finalizedBy restoreSageConstants


Why was this line removed?

fixed (commented out when I was trying to get the commit number code working again after move to ubuntu 22.04)

Narflex · 2025-03-13T18:46:23Z

All the latest changes look good. I just want to build it locally and install it on my server now for a couple days to test it. :) I'll also trigger the CI runs as well. If all goes well, then I'll merge it in a few days.

Narflex · 2025-03-13T19:24:11Z

And for some reason I'm having problems building this locally...given other people have done plenty of testing on it, I'll just merge it once Andy does the required STV changes because the property names have been changed.

Narflex · 2025-03-13T21:51:07Z

Just figured out my local build issue (wrong version of Java) so now I have it built and will also test it...now that it's all been merged. :)

jusjoken force-pushed the master branch 15 times, most recently from 0305774 to 978d1e6 Compare March 11, 2025 00:02

Narflex reviewed Mar 11, 2025

View reviewed changes

Narflex approved these changes Mar 11, 2025

View reviewed changes

Narflex reviewed Mar 11, 2025

View reviewed changes

build.gradle Show resolved Hide resolved

Narflex reviewed Mar 11, 2025

View reviewed changes

Narflex approved these changes Mar 11, 2025

View reviewed changes

jusjoken added 2 commits March 11, 2025 17:23

SD EPG changes to fix SD lockout issues

167c011

SD EPG Updates - added seeker/duration_for_watchdog to allow users to…

f8c76d6

… extend this process wait if causing issues with large libraries SD EPG Updates - seeker/duration_for_watchdog - fixed debug message to include valid duration

jusjoken force-pushed the master branch 2 times, most recently from 8cff323 to cbb0c7a Compare March 11, 2025 23:39

Updates to build.gradle and Github actions to support change to ubunt…

9dc22cf

…u 22.04

jusjoken force-pushed the master branch 3 times, most recently from e5cba2e to 9dc22cf Compare March 12, 2025 00:17

Release for version 9.2.9 [ci release]

0c3aa08

Narflex reviewed Mar 12, 2025

View reviewed changes

Fixes after PR review

ff61323

jusjoken changed the title ~~SD EPG fixes~~ SD EPG fixes [ci release] Mar 13, 2025

Narflex merged commit 2496057 into google:master Mar 13, 2025
2 checks passed

SD EPG fixes [ci release] #493

SD EPG fixes [ci release] #493

Uh oh!

Conversation

jusjoken commented Mar 8, 2025

Uh oh!

google-cla bot commented Mar 8, 2025

Uh oh!

Narflex left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Narflex commented Mar 11, 2025

Uh oh!

Narflex left a comment

Choose a reason for hiding this comment

Uh oh!

Narflex left a comment

Choose a reason for hiding this comment

Uh oh!

Narflex left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Narflex left a comment

Choose a reason for hiding this comment

Uh oh!

Narflex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Narflex commented Mar 13, 2025

Uh oh!

Narflex commented Mar 13, 2025

Uh oh!

Uh oh!

Narflex commented Mar 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development