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.

Sign up for GitHub

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

Jump to bottom

アンテナTLの投稿が200件もあるように見えない #11404

Closed
tamaina opened this issue Jul 27, 2023 · 26 comments
Closed

アンテナTLの投稿が200件もあるように見えない #11404

tamaina opened this issue Jul 27, 2023 · 26 comments
Assignees
@syuilo
Labels
🧩API Issues regarding the interface between the server and the client. 🐛Bug Unexpected behavior packages/backend Server side specific issue/PR

Comments

@tamaina
Copy link
Contributor

tamaina commented Jul 27, 2023 •
edited
Loading

アンテナTLが保持するノートは200件までとハードコーディングされているが、実際に保持されているのは数十件のように見える

misskey/packages/backend/src/core/AntennaService.ts

Line 102 in 8b74d15

redisPipeline.xadd(

p1.a9z.dev, m.tkngh.jpなどで確認
@tamaina tamaina added the ⚠️bug? This might be a bug label Jul 27, 2023
@tamaina tamaina changed the title アンテナの投稿は アンテナTLの投稿が200件もあるように見えない Jul 27, 2023
@tamaina
Copy link
Contributor Author

tamaina commented Jul 27, 2023

一番最後に見えるのがリモートの古い投稿である場合が多いため、ストリームで流れてきた古いidの投稿をうまく比較できていない可能性がある？

@tamaina tamaina added 🐛Bug Unexpected behavior 🧩API Issues regarding the interface between the server and the client. and removed ⚠️bug? This might be a bug labels Jul 27, 2023
@tamaina
Copy link
Contributor Author

tamaina commented Jul 27, 2023

解決策がわからない

XADD-XREVRANGEをやめる必要すらあるかもしれない

@tamaina
Copy link
Contributor Author

tamaina commented Jul 27, 2023

200件しかないなら都度全部読み出してJavaScript側で並び替え・切り出ししてもいいかも

@syuilo
Copy link
Member

syuilo commented Jul 29, 2023

投稿IDとは別にアンテナページング用IDを返すとか

@tamaina tamaina mentioned this issue Oct 9, 2023
Closed
@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

一番最後に見えるのがリモートの古い投稿である場合が多いため、ストリームで流れてきた古いidの投稿をうまく比較できていない可能性がある？

これっぽい

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

うーーーーーーーーーーーーーむ

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

200件しかないなら都度全部読み出してJavaScript側で並び替え・切り出ししてもいいかも

これかなあ

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

IDしか入ってないからメモリ的には全然大丈夫

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

ただstreamsを使うメリットが減る

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

list使えないか検討する

@tkmrgit tkmrgit mentioned this issue Oct 9, 2023
Closed
@syuilo syuilo self-assigned this Oct 9, 2023
@syuilo syuilo added the packages/backend Server side specific issue/PR label Oct 9, 2023
@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

あー

200件しかないなら都度全部読み出してJavaScript側で並び替え・切り出ししてもいいかも

したとしても、

  1. データベースに新しい順で A, B, C, D, E, Y, Z の投稿があり、Redis上には A, B, C がキャッシュされているとする
  2. 古い投稿 X が来て、データベースには A, B, C, D, E, X, Y, Z があり、Redis上(ソート後)には A, B, C, X がある状態になる
  3. TL取得時、Redisからは A, B, C, X が取得され、それがクライアントに返る
  4. ページネーションが行われると、X の ID が untilId として採用される
  5. Redisには X より古いものはないので、データベースにフォールバックされて X より古い Y, Z が返される
  6. D, E が抜ける

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

tasukete

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

古い投稿 X が来たときに、Redisにキャッシュされている最古のIDより新しい場合だけpushするようにすれば解決するけど、いちいち確認するのはパフォーマンス上のデメリットがある

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

IDが1分以内に生成されたものでない場合はRedisに追加しないとかかなぁ
抜けが発生する可能性は残るけど確立としてはかなり低くなる

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

少なくとも今よりはるかにマシ

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

いや問題があるな

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

いやないかも

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

Redis streamsが途中にエントリ挿入できるようになってくれれば全てが解決するんだけど

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

古い投稿 X が来たときに、Redisにキャッシュされている最古のIDより新しい場合だけpushするように

と

IDが1分以内に生成されたものでない場合はRedisに追加しない

を組み合わせて、

IDが1分以内に生成されたものは特にチェックせずRedisに突っ込み、そうでないならRedisの最古のIDを調べてそれより新しかった場合だけ突っ込む

ようにすればほぼ完璧かも

@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

来たものの99%は1分以内に投稿されたものだろうから最古IDをチェックしなければならないケースはほとんどないと思われるためパフォーマンスへの影響はほぼなくなる

@syuilo syuilo closed this as completed in 0f367da Oct 9, 2023
@syuilo
Copy link
Member

syuilo commented Oct 9, 2023

とりあえず

IDが1分以内に生成されたものでない場合はRedisに追加しない

だけ実装

@tkmrgit
Copy link

tkmrgit commented Oct 9, 2023

hjehet
beta12

@tkmrgit
Copy link

tkmrgit commented Oct 9, 2023

netejney
beta 12 私のアカウントの場合

とりあえず #11984 のように遡りが止まるのは直った

@fruitriin
Copy link
Contributor

アンテナ、なんかlimit 30で31件かえってくることがあって、その31件目はめちゃくちゃ古いPostで、そうするとuntillIdがめっちゃ古いノートを指すからなにもとってこれてないかも

syuilo added a commit that referenced this issue Oct 9, 2023
@syuilo
enhance(backend): RedisへのTLの構築をListで行うように
0bb0c32 
#11404
@tkmrgit
Copy link

tkmrgit commented Oct 10, 2023 •
edited
Loading

beta15 on みすてむずで修正確認。もう大丈夫っぽい！(ユーザーTL確認、アンテナは追加修正が来たようなので後ほど検証が必要かも)

@kakkokari-gtyih kakkokari-gtyih mentioned this issue Oct 10, 2023
Merged
@syuilo
Copy link
Member

syuilo commented Oct 20, 2023

sorted set使う(idをスコアとして扱う)手もありそう

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@syuilo syuilo

Labels
🧩API Issues regarding the interface between the server and the client. 🐛Bug Unexpected behavior packages/backend Server side specific issue/PR
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@syuilo @tamaina @fruitriin @tkmrgit