fix broken timelines

Johann150 commented

2022-09-02 15:38:13 +00:00

Owner

handle note visibility in SQL

This allows to check visibility recursively, which should hopefully solve #70 because the visibility is checked properly.

handle note visibility in SQL This allows to check visibility recursively, which should hopefully solve #70 because the visibility is checked properly.

Johann150 changed title from ~~WIP: fix broken timelines~~ to fix broken timelines

2022-09-02 17:40:01 +00:00

norm commented

2022-09-04 22:31:53 +00:00

Owner

Think I mentioned it in IRC a few times, but this does seem to cause a lot more timeouts especially on user profiles and doing searches.

norm commented

2022-09-19 19:18:11 +00:00

Owner

Wondering if memoizing the recursive results somehow would help with performance...

Johann150 commented

2022-09-19 19:51:08 +00:00

Author

Owner

For the search I think the problem is that the visibility query is being applied at the same time as the actual search. If we could instead first do the search and then apply the visibility query only to the limited result of that I think we should see a significant performance improvement.

Perhaps similar for the user profiles: First select the posts, then apply the visibility query.

In general it might be a good idea to refactor generateVisibilityQuery to somehow run the "main" query as a subselect and then apply the visibility query, although I'm not sure if or how that is possible in TypeORM.

For the search I think the problem is that the visibility query is being applied at the same time as the actual search. If we could instead first do the search and then apply the visibility query only to the limited result of that I think we should see a significant performance improvement. Perhaps similar for the user profiles: First select the posts, then apply the visibility query. In general it might be a good idea to refactor `generateVisibilityQuery` to somehow run the "main" query as a subselect and then apply the visibility query, although I'm not sure if or how that is possible in TypeORM.

norm commented

2022-09-19 20:04:26 +00:00

Owner

In general it might be a good idea to refactor generateVisibilityQuery to somehow run the "main" query as a subselect and then apply the visibility query, although I'm not sure if or how that is possible in TypeORM.

From what I can tell, you'd make another query builder for the subquery, then use the getQuery() method to get the generated SQL and use that in the main query builder in place of a table: https://typeorm.io/select-query-builder#using-subqueries

> In general it might be a good idea to refactor `generateVisibilityQuery` to somehow run the "main" query as a subselect and then apply the visibility query, although I'm not sure if or how that is possible in TypeORM. From what I can tell, you'd make another query builder for the subquery, then use the `getQuery()` method to get the generated SQL and use that in the main query builder in place of a table: https://typeorm.io/select-query-builder#using-subqueries

Johann150 commented

2022-09-19 20:39:22 +00:00

Author

Owner

I don't think that works here because what I wanted to do in SQL is something like e.g.

SELECT * FROM (
	SELECT * FROM "note" WHERE "text" LIKE '%blah%'
) "note" WHERE note_visible("note"."id", :userId);

I.e. select stuff not from the notes table itself but from the subquery. The example you linked does not do that, it innerJoins the subquery.

Apparently query builders have a from method where you can also make use of subqueries, see https://typeorm.io/select-query-builder#using-subqueries. However, both of the examples that use the "from subquery" functionality also use getRawMany at the end. Would have to test if getMany (i.e. not raw) would also work.

I don't think that works here because what I wanted to do in SQL is something like e.g. ```SQL SELECT * FROM ( SELECT * FROM "note" WHERE "text" LIKE '%blah%' ) "note" WHERE note_visible("note"."id", :userId); ``` I.e. select stuff not from the notes table itself but from the subquery. The example you linked does not do that, it `innerJoin`s the subquery. - - - Apparently query builders have a `from` method where you can also make use of subqueries, see <https://typeorm.io/select-query-builder#using-subqueries>. However, both of the examples that use the "from subquery" functionality also use `getRawMany` at the end. Would have to test if `getMany` (i.e. not raw) would also work.

Johann150 force-pushed missing-timeline from fd5b46c65a to 8a90dfa60b

2022-10-28 21:50:03 +00:00

Compare

Johann150 commented

2022-10-28 21:52:12 +00:00

Author

Owner

Now rewritten to do it in two stages. By testing it at least seems to not completely break. Question is of course if it still has performance problems. Can you test again?

norm commented

2022-10-28 23:07:40 +00:00

Owner

Seems to work now. No performance issues as far as I can tell.

🎉 1

norm commented

2022-10-28 23:37:36 +00:00

Owner

Timeline still stops loading after a certain point, nvm.

Johann150 commented

2022-10-28 23:38:47 +00:00

Author

Owner

Can you check in the networking tab how many notes were in the last API response for the timeline endpoint?

norm commented

2022-10-28 23:41:48 +00:00

Owner

From the last timeline response:

[norm@mikoto ~]$ jq '. | length' response.json 
30

From the last `timeline` response: ``` [norm@mikoto ~]$ jq '. | length' response.json 30 ```

👀 1

Johann150 commented

2022-10-28 23:47:20 +00:00

Author

Owner

Hmm, now I'm even more confused than before. I think that means at least this problem is fixed because the number of notes returned is as intended.

But there is apparently another bug in the client somewhere? 🥴 No idea.

Hmm, now I'm even more confused than before. I think that means at least this problem is fixed because the number of notes returned is as intended. But there is apparently another bug in the client somewhere? 🥴 No idea.

norm commented

2022-10-29 00:45:26 +00:00

Owner

It looks like though that previous responses from the timeline endpoint have 31 notes.

Seems like somewhere the server doesn't send the extra note that the client requests and therefore the client thinks there's nothing left to fetch.

It looks like though that previous responses from the timeline endpoint have 31 notes. Seems like somewhere the server doesn't send the extra note that the client requests and therefore the client thinks there's nothing left to fetch.

Johann150 commented

2022-10-29 00:46:08 +00:00

Author

Owner

Ah right, it should be 31, so it's apparently still the same issue. My bad.

👍 1

vib commented

2022-11-20 13:38:57 +00:00

Contributor

Not sure if my random input will be of any value, but here are some things:
I hope my wording won't be too tangled.

Right now a new user on my instance created an account and only followed another user who had only 2 posts to show on the Home timeline. So the timeline wouldn't load at all from the start, the request for getting 11 posts as the timeline page is loaded would return 500.

I created a test account myself to poke around and indeed I could reproduce their issue. Resending the request with limit: 2 instead of 11 will succeed. So it seems it fails if the limit is higher than the posts it has to show. But I am not sure if this is a separate different issue from this.

But then I also followed only my backup alt account. The request for the top of the timeline (with limit: 11) worked. but, scrolling down, the next request for the continuation of the timeline (with {"limit":31,"untilId":"96qo2d643g"} in the body) would time out. But if I sent this same request with limit: 10 it would succeed to respond... 11 and higher would fail.
I mentioned it is my backup account because populating the timeline with only my main one would make the timeline work fine. Now what difference between the two would make the difference? I don't know. The backup is a remote account with posts further between each other chronologically, since I post rarer there, while my main account is local and more dense in posts.

I didn't get to look into what happens exactly on the server side, with debug logs, but I can try that.

Not sure if my random input will be of any value, but here are some things: I hope my wording won't be too tangled. Right now a new user on my instance created an account and only followed another user who had only 2 posts to show on the Home timeline. So the timeline wouldn't load at all from the start, the request for getting 11 posts as the timeline page is loaded would return 500. I created a test account myself to poke around and indeed I could reproduce their issue. Resending the request with `limit: 2` instead of 11 will succeed. So it seems it fails if the limit is higher than the posts it has to show. But I am not sure if this is a separate different issue from this. But then I also followed only my backup alt account. The request for the top of the timeline (with `limit: 11`) worked. but, scrolling down, the next request for the continuation of the timeline (with `{"limit":31,"untilId":"96qo2d643g"}` in the body) would time out. But if I sent this same request with limit: 10 it would succeed to respond... 11 and higher would fail. I mentioned it is my backup account because populating the timeline with only my main one would make the timeline work fine. Now what difference between the two would make the difference? I don't know. The backup is a remote account with posts further between each other chronologically, since I post rarer there, while my main account is local and more dense in posts. I didn't get to look into what happens exactly on the server side, with debug logs, but I can try that.

👀 1

Johann150 commented

2022-11-20 15:00:31 +00:00

Author

Owner

I couldn't reproduce this on a testing instance, so if you could check if there are any log entries related to this would be good. Although I suspect it might be a database timeout for one reason or another.

Johann150 added the

fix

label 2022-12-23 10:28:49 +00:00

vib commented

2023-02-04 18:17:26 +00:00

Contributor

(Sorry I came back so late)
Yes it's because of a database timeout, many queries on it are slow and I'm trying to troubleshoot that.
But either way I think my problem is unrelated to the original issue of this thread. Which, to add, doesn't occur on 3rd party clients like Kaiteki while on the web client it would. So it may be related to the client.
I could open a new issue about the queries that are slow and timing out for me and look deeper into it

(Sorry I came back so late) Yes it's because of a database timeout, many queries on it are slow and I'm trying to troubleshoot that. But either way I think my problem is unrelated to the original issue of this thread. Which, to add, doesn't occur on 3rd party clients like Kaiteki while on the web client it would. So it may be related to the client. I could open a new issue about the queries that are slow and timing out for me and look deeper into it

Johann150 force-pushed missing-timeline from 8a90dfa60b to d7d10ccd60

2023-02-20 06:42:51 +00:00

Compare

Johann150 force-pushed missing-timeline from 23f811ec48 to aece832c86

2023-02-20 19:53:41 +00:00

Compare

Johann150 manually merged commit 0a7352eda9 into main

2023-02-20 20:10:13 +00:00

Johann150 deleted branch missing-timeline

2023-02-20 20:10:31 +00:00

Johann150 referenced this pull request

2023-02-20 20:13:02 +00:00

Can't scroll back past a limited number of posts #70

fix broken timelines #102