Track additional statistics#6390
Conversation
…sadev/lemmy into additional-statistics-scheduled-task
| ADD COLUMN total_communities integer; | ||
|
|
||
| ALTER TABLE local_site | ||
| ADD COLUMN user_retention_percent integer; |
There was a problem hiding this comment.
All the percentages/rates should probably be floats.
| ALTER TABLE local_site | ||
| ADD COLUMN local_post_english_percent integer; |
There was a problem hiding this comment.
Not sure how useful this metric would be. I spose it could be useful for servers who are multi-lingual, and wanting to try to get rid of english usage.
There was a problem hiding this comment.
Yeah it shouldnt be limited to a single language, but have usage percentage for every language. Maybe with a new column language.usage_percentage.
There was a problem hiding this comment.
I added language_usage_percent jsonb column. It will contain something like:
{
"en" : "30.10",
"und": "10.00",
"de": "59.90"
}There was a problem hiding this comment.
I'm not a fan of slamming json into sql, we really should try to avoid that. Either add a new table, or better yet just remove this.
There was a problem hiding this comment.
Best put it in the existing language table.
- Rename `users/posts/comments/communities` to `local_users/local_posts/local_comments/local_communities` to distinguish from the new total_* columns - Add `NOT NULL DEFAULT 0` to all new statistics columns (linked_instances, total_*, rates) instead of nullable - Update triggers.sql, nodeinfo.rs, convert.rs, and tests to use new column names
malsadev
changed the title
|
Clippy is failing, see here or run ./scripts/lint.sh |
| ADD COLUMN linked_instances integer NOT NULL DEFAULT 0; | ||
|
|
||
| ALTER TABLE local_site | ||
| ADD COLUMN total_posts integer NOT NULL DEFAULT 0; |
There was a problem hiding this comment.
FYI you can combine most or all of these into a single statement, but doesnt make much difference.
ALTER TABLE local_site
ADD COLUMN linked_instances integer NOT NULL DEFAULT 0,
ADD COLUMN total_posts integer NOT NULL DEFAULT 0;
| ALTER TABLE local_site | ||
| ADD COLUMN language_usage_percent jsonb NOT NULL DEFAULT '{}'::jsonb; |
There was a problem hiding this comment.
Better do
| ALTER TABLE local_site | |
| ADD COLUMN language_usage_percent jsonb NOT NULL DEFAULT '{}'::jsonb; | |
| ALTER TABLE language | |
| ADD COLUMN usage_percent float default 0; |
| ADD COLUMN accepted_signups_rate integer NOT NULL DEFAULT 0; | ||
|
|
||
| ALTER TABLE local_site | ||
| ADD COLUMN failed_signups_rate integer NOT NULL DEFAULT 0; |
There was a problem hiding this comment.
The rates should be float. Also they are not currently written by the scheduled task.
| update(local_site::table) | ||
| .set(local_site::total_posts.eq(total_post_count)) | ||
| .execute(conn) | ||
| .await?; |
There was a problem hiding this comment.
Only make a single update query using LocalSiteUpdateForm. Or you can update multiple columns with a tuple like this:
update(local_site::table)
.set((
local_site::total_posts.eq(total_post_count),
local_site::total_comments.eq(total_comment_count),
...
))
.execute(conn)
.await?;
| .select(count_star()) | ||
| .first::<i64>(conn) | ||
| .await | ||
| .map(i32::try_from)??; |
There was a problem hiding this comment.
Similar to update_local_user_count this should filter out deleted and banned users. Same for communities, posts and comments. There deleted and removed should be filtered out.
| post_count.lang_code, | ||
| Value::Number( | ||
| serde_json::Number::from_f64( | ||
| (post_count.post_count as f64 * 10000.0 / f64::from(local_post_count)).round() / 100.0, |
There was a problem hiding this comment.
This is a bit confusing with all the conversions, should be simpler when switching to language.usage_percent as mentioned below. And why multiply with 10000 then divide by 100?
| update(local_site::table) | ||
| .set(local_site::language_usage_percent.eq(Value::Object(post_counts))) | ||
| .execute(conn) | ||
| .await?; |
There was a problem hiding this comment.
This also doesnt need a separate update query. Instead return the calculated usage percentage, and write it in a single query in update_stats
| .select(count_star()) | ||
| .first::<i64>(conn) | ||
| .await | ||
| .map(i32::try_from)??; |
There was a problem hiding this comment.
Please add a method FederatedInstanceView::count, then you can reuse FederatedInstanceView::joins instead of writing the same thing again.
|
@Nutomic I appreciate the new wave of feedback. I'm cleaning up the PR. |
3 participants
Issue: #6288
Summary of changes:
In
local_sitetable:posts,users, (and others) tolocal_posts,local_users, etc.total_posts,total_users, etclanguage_usage_percentjsonbcolumn with a non null constraint and default value{}There were tests that were exercising some trigger logic in the db so I fixed the db triggers/functions here.
update_statsmethod which runs some basics queries and updateslocal_sitetable and added it to the daily scheduled tasksprocess_language_breakdown(runs as part ofupdate_stats) which calculates the percentage breakdown of posts per language tag and updates the newly addedlanguage_usage_percentfield inlocal_sitetabletest_update_statsandtest_process_language_breakdowntests--features fullflag to./scripts/dump_schema.rsOutstanding items: