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

Improve playback statistics #4339

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/invidious/jobs/statistics_refresh_job.cr
Expand Up @@ -27,6 +27,12 @@ class Invidious::Jobs::StatisticsRefreshJob < Invidious::Jobs::BaseJob
"playback" => {} of String => Int64 | Float64,
}

# Latches the playback success stats from before statistics gets refreshed
# Used to ensure that the object won't get reset back to an empty object
LATCHED_PLAYBACK_STATS = {
"playback" => {} of String => Int64 | Float64,
}

private getter db : DB::Database

def initialize(@db, @software_config : Hash(String, String))
Expand Down Expand Up @@ -65,6 +71,7 @@ class Invidious::Jobs::StatisticsRefreshJob < Invidious::Jobs::BaseJob
}

# Reset playback requests tracker
LATCHED_PLAYBACK_STATS["playback"] = STATISTICS["playback"].as(Hash(String, Int64 | Float64))
STATISTICS["playback"] = {} of String => Int64 | Float64
Copy link
Member

Choose a reason for hiding this comment

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

Why not just moving this line (the reset) to load_initial_stats and then never touch that element again?

Copy link
Member Author

Choose a reason for hiding this comment

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

load_initial_stats is only called once and we need to latch the old playback stats each time the statistics is refreshed.

def begin
load_initial_stats
loop do
refresh_stats
sleep 10.minute
Fiber.yield
end
end

end
end
2 changes: 2 additions & 0 deletions src/invidious/routes/api/v1/misc.cr
Expand Up @@ -19,6 +19,8 @@ module Invidious::Routes::API::V1::Misc
else
tracker["ratio"] = (success_count / (total_requests)).round(2)
end
else
return (Invidious::Jobs::StatisticsRefreshJob::STATISTICS.merge Invidious::Jobs::StatisticsRefreshJob::LATCHED_PLAYBACK_STATS).to_json
end
end

Expand Down
8 changes: 8 additions & 0 deletions src/invidious/videos/parser.cr
Expand Up @@ -89,6 +89,14 @@ def extract_video_info(video_id : String, proxy_region : String? = nil)
}
else
reason = nil

# Although technically not a call to /videoplayback, because we are counting requests
# in which YouTube returned a "this content is not available" video as a failure,
# we should also count requests that returned the correct video as a success
# in order to ensure a correct and accurate ratio
playback_stats = get_playback_statistic()
playback_stats["totalRequests"] += 1
playback_stats["successfulRequests"] += 1
end

# Don't fetch the next endpoint if the video is unavailable.
Expand Down