Refactor thread fetching into an iterative operation about kitsune

[aumetra]

To be fair, the whole ActivityPub fetching logic with the database mapping needs a major cleanup. It's in an abysmal state at the moment.

Maybe that cleanup can also add better testing to it finally.

[tesaguri]

I think the ideal handling of threads is to use an asynchronous job worker like Mastodon's ThreadResolveWorker instead of occupying a task until the thread is fully resolved, iterative or not.

Maybe this would require the database schema to be aware of replies whose in_reply_to_id is yet to be resolved.

[aumetra]

Yeah, good point. But even if it's run in a background task, I'd personally prefer an iterative operation since it won't exhaust the stack space because of function calls

[aumetra]

Maybe this would require the database schema to be aware of replies whose in_reply_to_id is yet to be resolved

Not sure if we need that. We could just set that to NULL if we haven't resolved that and then just enqueue a new job to resolve the thread on initial fetch

[tesaguri]

Yeah, good point. But even if it's run in a background task, I'd personally prefer an iterative operation since it won't exhaust the stack space because of function calls

I was assuming that the worker would only fetch a single post in a job and, well, recursively enqueue another job to the scheduler to fetch the reply until the thread is fully resolved, so that it won't exhaust the stack (and each job is reasonably short-lived). That's how Mastodon's worker is implemented IIUC.

We could just set that to NULL if we haven't resolved that and then just enqueue a new job to resolve the thread on initial fetch

Yes, I think we don't need this if we can hold a transaction over the whole thread resolution process, but otherwise, clients may observe a reply whose in_reply_to post haven't been resolved. Just making the in_reply_to_id to NULL in this case would make the client to recognise the post as a non-reply, which is not true.

Well, come to think of it, we don't keep a transaction over the whole process_new_object function today either, so this problem might not be inherent to the said worker approach anyway.