Give some time for "what's new" podcast search

[philippe44]

I'm not 100% sure but I've seen a few cases where the "what's new" on podcasts does not work: a first list is displayed and when clicking on one item, a different one is played.

Because queries are cached, the only rational explanation I have is that when there are a lot of feeds, the first swarm of requests is taking too much time and some timeout. Then, because LMS always redescend the branches to access a leaf, when the queries are re-done to actually play an item, the missing ones are executed and the returned list is different than the initial one and because it uses indexes from the initial list, we end up playing something else.

This PR tries to give a min time of 15s in anycase, but expands it above if we have more feeds, like 2 seconds per feed.

[michaelherger]

Do you believe (or know from experience) that the additional seconds would actually help? The problem I'm seeing is that the timeout with a lots of feeds can easily reach a minute or more. And that's not useful any more. I don't like timeout based solutions to these kinds of problems as they're always too long or too short.

I'd rather use a different caching strategy: instead of solely relying on the http cache introduce caching of the actual menu we're rendering. Then have the menu renderer re-use the rendered menu when descending down again for playback. This way at least the same incomplete list would be used to play back. While it might still be missing items, the playback would be more reliable, wouldn't it? As long as the user decides to play an item within the cache TTL. But that's a requirement anyway.

[philippe44]

Well.. you're 100% right but I'm becoming lazy. I moved away from caching the menu themselves as I did in the YT plugin for the benefit of caching the HTTP response. I don't know if I have the courage to do anything better than that or at best case hash the requested url and put a cache entry for it, valid only for a minute or so...

[michaelherger]

What about this:

diff --git a/Slim/Plugin/Podcast/PodcastIndex.pm b/Slim/Plugin/Podcast/PodcastIndex.pm
index 780653c92..1a8368aff 100644
--- a/Slim/Plugin/Podcast/PodcastIndex.pm
+++ b/Slim/Plugin/Podcast/PodcastIndex.pm
@@ -15,10 +15,15 @@ use Digest::SHA1 qw(sha1_hex);
 use MIME::Base64;
 use URI::Escape;
 
+use Slim::Utils::Cache;
 use Slim::Utils::Prefs;
 use Slim::Utils::Log;
 use Slim::Utils::Strings qw(string cstring);
 
+use constant NEWS_TTL => 300;
+use constant NEWS_CACHE_KEY => 'podcast_index_news';
+
+my $cache = Slim::Utils::Cache->new();
 my $prefs = preferences('plugin.podcast');
 my $log = logger('plugin.podcast');
 
@@ -72,6 +77,11 @@ sub getFeedsIterator {
 sub newsHandler {
 	my ($client, $cb, $args, $passthrough) = @_;
 
+	if (my $cached = $cache->get(NEWS_CACHE_KEY)) {
+		main::INFOLOG && $log->is_info && $log->info("Returning cached PodcastIndex news");
+		return $cb->({ items => $cached });
+	}
+
 	my $headers = getHeaders();
 	my @feeds = @{$prefs->get('feeds')};
 	my $count = scalar @feeds;
@@ -82,6 +92,15 @@ sub newsHandler {
 
 	$log->info("about to get updates for $count podcast feeds");
 
+	my $cb2 = sub {
+		if (!--$count) {
+			main::INFOLOG && $log->is_info && $log->info("Done getting updates");
+			$items = [ sort { $a->{date} < $b->{date} } @$items ];
+			$cache->set(NEWS_CACHE_KEY, $items, NEWS_TTL);
+			$cb->( { items => $items } );
+		};
+	};
+
 	foreach my $feed (@feeds) {
 		my $url = 'https://api.podcastindex.org/api/1.0/episodes/byfeedurl?url=' . uri_escape($feed->{value});
 		$url .= '&since=-' . $prefs->get('newSince')*3600*24 . '&max=' . $prefs->get('maxNew');
@@ -103,18 +122,11 @@ sub newsHandler {
 					};
 				}
 
-				unless (--$count) {
-					$items = [ sort { $a->{date} < $b->{date} } @$items ];
-					$cb->( { items => $items } );
-				}
+				$cb2->();
 			},
 			sub {
 				$log->warn("can't get new episodes for $url ", shift->error);
-				unless (--$count) {
-					$items = [ sort { $a->{date} < $b->{date} } @$items ];
-					$cb->( { items => $items } );
-				}
-
+				$cb2->();
 			},
 			{
 				cache => 1,

[philippe44]

Yes, the only thing then I would do is make the key not a constant but something like

my $key = "podcast-news-$count-" . $prefs->get('newSince') . '-' . $prefs->get('maxNew');

and maybe a TTL of a minute so that if user modifies (at least add/removes) the podcast list and/or the since/max, he has immediately the update

[philippe44]

Do you want me to change the PR or you do it directly?

[michaelherger]

I'll push my changes in a minute. Thanks!

[mherger]

Unfortunately our menu system will always come with a risk of taking action on an outdated hierarchy. But this is not specific to the podcast implementation. I hope the committed change has the expected effect.

[philippe44]

I'm a bit confused. Do you want me to then change the key definition? I mean I agree with the outdating "risk" but here when user specifically changes "since" or "max" he might be frustrated by the lack of change of result before 5 mins where a dynamic key is a low cost change to avoid that, no?

[philippe44]

Oh, I missed the "setChange"

[michaelherger]

My comment was just a general note. I took care of the prefs adding a prefs change handler. Nothing left to be done but discover a next issue 🙂.

[philippe44]

oh, just one thing, the HTTP timeout should then be set back to a fixed value (I think 30s b/c 15s is really low) and I really think the cache for the menu should be lower, like 1 min to give a chance for more to be read while minimizing the chance of outdated hierarchy, while the HTTP cache is much higher to minimize accesses