From c152a19624893beee0b0aa77b8676f0498027b44 Mon Sep 17 00:00:00 2001 From: Calvin Montgomery Date: Sat, 15 Jul 2017 14:41:37 -0700 Subject: [PATCH] Ignore library cached metadata when queueing The use of the channel library as a cache for metadata to avoid re-requesting metadata for known media is an optimization that dates back to 1.0. However, it doesn't have any TTL, is prone to bugs, and is of dubious value. This commit ignores the results of the library check when queueing a new video, opting to always re-request the metadata. This fixes a few bugs: * Google Drive metadata being lost when storing in library * Streamable metadata being lost when storing in library * Videos in the channel library that are now unavailable on their source website being queueable and then failing to play (e.g. deleted YouTube videos). In its place, a small fail-open check is left behind to emit metric counters on how many queues would have been cache-hits, to provide insight into whether a proper caching solution (i.e. one not tacked on top of the library) would be worth pursuing or not. This will be removed eventually. --- package.json | 2 +- src/channel/playlist.js | 40 +++++++++++++++++----------------------- src/database/channels.js | 3 ++- www/js/callbacks.js | 2 +- www/js/util.js | 5 +++-- 5 files changed, 24 insertions(+), 28 deletions(-) diff --git a/package.json b/package.json index d8b32671..188763ea 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "author": "Calvin Montgomery", "name": "CyTube", "description": "Online media synchronizer and chat", - "version": "3.39.6", + "version": "3.40.0", "repository": { "url": "http://github.com/calzoneman/sync" }, diff --git a/src/channel/playlist.js b/src/channel/playlist.js index a438939d..5afda666 100644 --- a/src/channel/playlist.js +++ b/src/channel/playlist.js @@ -8,6 +8,7 @@ var Flags = require("../flags"); var db = require("../database"); var CustomEmbedFilter = require("../customembed").filter; var XSS = require("../xss"); +import counters from '../counters'; const LOGGER = require('@calzoneman/jsli')('playlist'); @@ -328,6 +329,10 @@ PlaylistModule.prototype.handleQueue = function (user, data) { var id = data.id; var type = data.type; + if (type === "lib") { + LOGGER.warn("Outdated client: IP %s emitting queue with type=lib", + user.realip); + } if (data.pos !== "next" && data.pos !== "end") { return; @@ -450,36 +455,25 @@ PlaylistModule.prototype.queueStandard = function (user, data) { const self = this; this.channel.refCounter.ref("PlaylistModule::queueStandard"); + counters.add("playlist:queue:count", 1); this.semaphore.queue(function (lock) { var lib = self.channel.modules.library; if (lib && self.channel.is(Flags.C_REGISTERED) && !util.isLive(data.type)) { + // TODO: remove this check entirely once metrics are collected. lib.getItem(data.id, function (err, item) { if (err && err !== "Item not in library") { - error(err+""); - self.channel.refCounter.unref("PlaylistModule::queueStandard"); - return lock.release(); - } - - // YouTube livestreams transition to becoming regular videos, - // breaking the cached duration of 0. - // In the future, the media cache should be decoupled from - // the library and this will no longer be an issue, but for now - // treat 0-length yt library entries as non-existent. - if (item !== null && item.type === "yt" && item.seconds === 0) { - data.type = "yt"; // Kludge -- library queue has type: "lib" - item = null; - } - - if (item !== null) { - /* Don't re-cache data we got from the library */ - data.shouldAddToLibrary = false; - self._addItem(item, data, user, function () { - lock.release(); - self.channel.refCounter.unref("PlaylistModule::queueStandard"); - }); + LOGGER.error("Failed to query for library item: %s", String(err)); + } else if (err === "Item not in library") { + counters.add("playlist:queue:library:miss", 1); } else { - handleLookup(); + // temp hack until all clients are updated. + // previously, library search results would queue with + // type "lib"; this has now been changed. + data.type = item.type; + counters.add("playlist:queue:library:hit", 1); } + + handleLookup(); }); } else { handleLookup(); diff --git a/src/database/channels.js b/src/database/channels.js index f6f1c6ef..dead5225 100644 --- a/src/database/channels.js +++ b/src/database/channels.js @@ -431,7 +431,8 @@ module.exports = { bitrate: media.meta.bitrate, codec: media.meta.codec, scuri: media.meta.scuri, - embed: media.meta.embed + embed: media.meta.embed, + direct: media.meta.direct }); db.query("INSERT INTO `channel_libraries` " + diff --git a/www/js/callbacks.js b/www/js/callbacks.js index 5f679797..86a74362 100644 --- a/www/js/callbacks.js +++ b/www/js/callbacks.js @@ -879,7 +879,7 @@ Callbacks = { generator: function (item, page, index) { var li = makeSearchEntry(item, false); if(hasPermission("playlistadd") || hasPermission("deletefromchannellib")) { - addLibraryButtons(li, item.id, data.source); + addLibraryButtons(li, item, data.source); } $(li).appendTo($("#library")); }, diff --git a/www/js/util.js b/www/js/util.js index a10ce0c9..ed67e79c 100644 --- a/www/js/util.js +++ b/www/js/util.js @@ -1084,12 +1084,13 @@ function clearSearchResults() { } } -function addLibraryButtons(li, id, source) { +function addLibraryButtons(li, item, source) { var btns = $("
").addClass("btn-group") .addClass("pull-left") .prependTo(li); - var type = (source === "library") ? "lib" : source; + var id = item.id; + var type = item.type; if(hasPermission("playlistadd")) { if(hasPermission("playlistnext")) {