Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

merge: upstream #73

Merged
merged 151 commits into from
Oct 14, 2023
Merged

merge: upstream #73

merged 151 commits into from
Oct 14, 2023

Conversation

Mar0xy
Copy link
Contributor

@Mar0xy Mar0xy commented Oct 13, 2023

This merges all latest commits as well as the timeline change over to redis cache as it now keeps track of global and users messages again

syuilo and others added 30 commits October 4, 2023 08:46
* fix(frontend): クライアント設定から13.7.0で削除されたチャット機能に関するサウンド設定を削除

* fix(frontend): 各localesからsfx/chat sfx/chatbgを削除
* chore: renoteに関するチェックをまとめる

* fix: ダイレクト投稿をrenoteできる

* fix(frontend): 自分のダイレクト投稿をrenoteできる

* docs(changelog): ダイレクト投稿をリノートできてしまう

* fix lint

* chore(backend): visibilityに関するエラーをApi Errorとして返す
@dakkar
Copy link
Contributor

dakkar commented Oct 13, 2023

ok, so after these changes:

  • scrollback is limited by cache size, I can't scroll back as far as I want
  • since the timeline cache starts empty, users will not be shown (in their timelines) notes from before the upgrade
  • unless a user has read all notes up to the moment of the upgrade, they'll miss some notes
    • yes, the user can go through each person they follow and check their recent notes…

please tell me I'm wrong…

@Mar0xy
Copy link
Contributor Author

Mar0xy commented Oct 13, 2023

  1. is correct though you could inflate the cache to an extreme amount but i dont know what would happen
  2. yes though i am confused due to the fact that the update note states "temporarily"
  3. don't know what is meant with that

I also want to point out I was basically forced to pull this update cause otherwise keeping up with upstream would have become impossible as I spent about 2 hours on this just due to the timeline change was initially missing

@Mar0xy
Copy link
Contributor Author

Mar0xy commented Oct 13, 2023

And the recent changes also really brought good things with them as well like a better CW button

image

@Mar0xy
Copy link
Contributor Author

Mar0xy commented Oct 13, 2023

Update on user notes just checked them it seems they appear after a while again on a users profile which explains the "temporarily" statement:

image

@dakkar
Copy link
Contributor

dakkar commented Oct 13, 2023

I also want to point out I was basically forced to pull this update cause otherwise keeping up with upstream would have become impossible as I spent about 2 hours on this just due to the timeline change was initially missing

apologies, I really didn't mean to sound antagonistic in any way, or like I was demeaning your work! I very much appreciate the effort and care you're putting into Sharkey!

let me try to clarify my last question:

  • an instance runs Sharkey before this PR is merged
  • user X on that instance follows user Y
  • at 12:00, a note is created by user Y
    • that note would show up on user X's home timeline
  • user X is not looking at their home timeline, so doesn't see the note
  • at 12:05, the instance is upgraded to Sharkey after this PR is merged
  • at 12:10, user X looks at their home timeline
  • will user X see, in their home timeline, the note that user Y created at 12:00?

@Mar0xy
Copy link
Contributor Author

Mar0xy commented Oct 13, 2023

The user would not see the note in their home timeline initially due to the change to redis cache only if they view the user themself then they would see it.

@dakkar
Copy link
Contributor

dakkar commented Oct 13, 2023

ok, so it is as bad as I thought.

I'm all in favour of caching, but a cache without a back-filling mechanism is… not a cache, it's a lossy filter.

(and yes, I understand that it's not something "we" can practically fix…)

@Mar0xy
Copy link
Contributor Author

Mar0xy commented Oct 13, 2023

"we" can practically fix

Well if someone could figure out how to write a script that imports all notes from the DB into the redis cache then it could be fixed but I am not clever enough to do this stuff

@dakkar
Copy link
Contributor

dakkar commented Oct 13, 2023

impressive that api/channel/timeline has a "fallback to database" https://github.com/transfem-org/Sharkey/pull/73/files#diff-9de864eae560189a50ee3137979b3b9a32baed7da582458b552096b5b465e5dfR129 when the cache doesn't contain the notes that the request asks for, but api/notes/timeline (and the other timelines under api/notes/) doesn't ☹ great job, upstream!

also, possible warnings: since all the timelines of all users and channels get stored in Redis, I suspect that this new code will overall take a lot more RAM… like, if you have 100 inactive users, the current code does nearly nothing about them, but the new code will populate their timelines in Redis

@Mar0xy
Copy link
Contributor Author

Mar0xy commented Oct 13, 2023

if you have 100 inactive users

This will also depend on if they even follow anyone in the first place as most inactive users are accounts that got created and never used or people that really just reacted atleast once to a post.

@dakkar
Copy link
Contributor

dakkar commented Oct 13, 2023

fair!

@Mar0xy Mar0xy merged commit 2d78ec4 into develop Oct 14, 2023
9 checks passed
@Mar0xy Mar0xy deleted the merge/upstream branch October 14, 2023 01:42
This was referenced Oct 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.