New images stuck on "Thumbnailing..."

[Chiiruno]

Uploading a previously hashed image will work fine, but it seems like the server isn't accepting new images. I don't see anything on the commit list that's really related, same for thumbnailer, so could this be yet another cloudflare issue? Otherwise, perhaps captchouli integration is somehow blocking new images? Doesn't seem like it's just me this is happening to.

[Chiiruno]

This might also be a server-client issue, perhaps an invalid value is getting passed to websockets and isn't printing it out as a bug in the terminal? The client clearly has no way to detect it. Could you try restarting the server and see if that fixes it? If it does, that confirms at least a similar corruption.

[bakape]

Uploads don't go through CF. I might have fucked up with the last deployment.

On Wed, 28 Nov 2018, 13:28 チルノ <notifications@github.com wrote:

This might also be a server-client issue, perhaps an invalid value is getting passed to websockets and isn't printing it out as a bug in the terminal? The client clearly has no way to detect it. Could you try restarting the server and see if that fixes it? If it does, that confirms at least a similar corruption.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bakape/meguca/issues/932#issuecomment-442414722, or mute the thread https://github.com/notifications/unsubscribe-auth/AHfPsKJ5TovTGuMGZWo_mCU0imh8lfqjks5uznNHgaJpZM4Y3Y8e .

[bakape]

Will test this in around 4 hours.

On Wed, 28 Nov 2018, 14:36 Janis Petersons <bakape@gmail.com wrote:

Uploads don't go through CF. I might have fucked up with the last deployment.

On Wed, 28 Nov 2018, 13:28 チルノ <notifications@github.com wrote:

This might also be a server-client issue, perhaps an invalid value is getting passed to websockets and isn't printing it out as a bug in the terminal? The client clearly has no way to detect it. Could you try restarting the server and see if that fixes it? If it does, that confirms at least a similar corruption.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bakape/meguca/issues/932#issuecomment-442414722, or mute the thread https://github.com/notifications/unsubscribe-auth/AHfPsKJ5TovTGuMGZWo_mCU0imh8lfqjks5uznNHgaJpZM4Y3Y8e .

[bakape]

Okay, one thread was stuck in an infinite loop. Since all thumbnailing is now done on only one goroutine, this explains the blocking of thumbnailing. Basically, we need to localize all thumbnailing to one goroutine (or thread?) and kill it, if it times out. Not sure how to do that yet.

[Chiiruno]

Since all thumbnailing is now done on only one goroutine

Why would you do this? I would have a minimum of at least 3-4 goroutines for meguca at it's current size, if we got 1000 newfags all of a sudden thumbnailing would grind to a halt.

Timeouts should be easy, have a mutex locked boolean (Or an atomic boolean if you can do that) that only unlocks when the goroutine is over (So, unlock it at the end of the thumbnailing process), and if it isn't unlocked let's say in 4 seconds, kill the goroutine and retry up to X times, if past X, print error.

[Chiiruno]

You could probably have multiple of these booleans for multiple goroutines, a scalable system might even be possible where if there are say 100 active users, we have like 10 goroutines for thumbnailing available. Maybe 5 might be more reasonable.

[bakape]

Why would you do this?

OOM races. It's a hotfix until we have a proper memory-aware scheduler. But, if we consider

memory scheduling in GC-ed languages

that might be never.

Timeouts should be easy, have a mutex locked boolean (Or an atomic boolean if you can do that) that only unlocks when the goroutine is over (So, unlock it at the end of the thumbnailing process), and if it isn't unlocked let's say in 4 seconds, kill the goroutine and retry up to X times, if past X, print error.

Yup, the question was how to do that in Go, but I seems you can do that with contexts. https://golang.org/pkg/context/#WithCancel

[bakape]

Also, maybe this should be integrated into the thumbnailer lib instead. Thoughts?

[Chiiruno]

This probably should be thumbnailer, that way hydron and captchouli can benefit too.

[Chiiruno]

Perhaps thread handling for thumbnailer should be handled in C++ instead, so that way a proper system can be implemented, since I doubt we'll see Google work on memory scheduling any time soon, and a custom one (if needed) in C++ might just be more efficient since thumbnailer is relatively small. It's been a while since I've worked with threads in C++, so maybe I got it all wrong, what do you think? Probably

[bakape]

You could probably have multiple of these booleans for multiple goroutines, a scalable system might even be possible where if there are say 100 active users, we have like 10 goroutines for thumbnailing available. Maybe 5 might be more reasonable.

Does not prevent OOM. Still has to be memory-aware. Could possibly be something like this:

init:
spawn 1 worker

thumbnailing:
if free_memory < 500 MB (tentative)
  queue on default worker and block
 else
  spawn 1 worker
  thumbnail
  destroy worker

Thoughts?

[bakape]

Perhaps thread handling for thumbnailer should be handled in C++ instead, so that way a proper system can be implemented, since I doubt we'll see Google work on memory scheduling any time soon, and a custom one (if needed) in C++ might just be more efficient since thumbnailer is relatively small. It's been a while since I've worked with threads in C++, so maybe I got it all wrong, what do you think?

We'd need to port almost all the current Go code in thumbnailer to C++ for that first, i think.

[Chiiruno]

That looks generally fine to me, can that be done in Go with Goroutines?

[bakape]

That looks generally fine to me, can that be done in Go with Goroutines?

Yes.

[Chiiruno]

Do we get a significant advantage with porting thumbnailer to C++ over Go for this case? (And some other adjacent cases, mainly hydron being memory hungry when importing)

[bakape]

Do we get a significant advantage with porting thumbnailer to C++ over Go for this case? (And some other adjacent cases, mainly hydron being memory hungry when importing)

Once we get rid of GM and do all thumbnailing operations through FFmpeg + some helper libs, that will let us avoid the horrorshow of copying massive buffers from Go to C and reduce FFI context switch costs somewhat. All source file reading can then be done through the https://github.com/bakape/thumbnailer/blob/master/ffmpeg.go#L200-L221 interface.

[bakape]

Anyway, this is for later. I have to get back to work an assignments. How's life on your side?

[Chiiruno]

Okay, so it looks like as long as we can axe GM, Go memory handling should be generally okay.

[Chiiruno]

Too much time and a modded minecraft addiction.

[bakape]

Wait, no. Contexts won't, if we are blocking in an infinite loop somewhere on the C side. How the fuck do I kill a goroutine?

[Chiiruno]

https://stackoverflow.com/questions/51941132/how-to-kill-goroutines Oh wow, this looks bad already. I'll look some more.

[bakape]

If there really is not, I think spawning threads in C is the only choice.

[Chiiruno]

https://stackoverflow.com/questions/42560109/how-do-i-kill-a-goroutine Looks like you can do a bit of a hack here.

[bakape]

Won't work. That assumes your code is littered with select conditional breakpoints. Seems like https://stackoverflow.com/a/13285445/2420900 is the way to go.

[bakape]

Related: https://gist.github.com/bryanhunter/3960915

[Chiiruno]

That's for C, are you thinking of killing goroutines from C? I didn't even know that was possible without violating some memory boundaries.

[bakape]

No, spawning C threads a workers. You can kill those.

[bakape]

Fucking Github.

[Chiiruno]

Yeah, I did that too once, there needs to be a confirmation thing. Spawning pthreads is better in my opinion too, but honestly I'd do it in C++ for ease of use and at some point, legibility.

[Chiiruno]

Ah, right now a lot of thumbnailer code is already in C though, so I guess this makes sense until something "better".

[bakape]

No point in using C++ as all the libs we use have C APIs. It would just be C with C++ locks then. Hmm, hows doe C++ deal with killing threads? Does it execute all the destructors in the thread?

AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA FFmpeg is full of reference-counted resources. How do you free those on thread kill?

[bakape]

I see. So this is why it's a popular opinion that complex concurrency necessitates garbage collection.

[Chiiruno]

However C++ deals with killing threads is likely cleaner than manually doing it with pthread, as well as spawning the threads themselves. You could have thumbnailer use C++, and just interface with the libs, you still get benefits of C++ from the spawning/killing of threads and maybe some other things.

Not sure how C++ deals with it all that well, but here's something: https://www.bo-yang.net/2017/11/19/cpp-kill-detached-thread

Does FFmpeg not have a system already in place for dealing with all that? Maybe just free from the top and see if it works in some testing?

[bakape]

However C++ deals with killing threads is likely cleaner than manually doing it with pthread, as well as spawning the threads themselves. You could have thumbnailer use C++, and just interface with the libs, you still get benefits of C++ from the spawning/killing of threads and maybe some other things.

You don't. This would be true only if FFmpeg was also written in C++. You would just use cleanup handlers just like in C.

Does FFmpeg not have a system already in place for dealing with all that?

pthread_cleanup_push()

[bakape]

you still get benefits of C++ from the spawning/killing of threads and maybe some other things

Except you don't. FFMPEG does not use C++ and therefore does not have destructors.

Does FFmpeg not have a system already in place for dealing with all that? Maybe just free from the top and see if it works in some testing?

See above.

[Chiiruno]

Ah naruhodo.

[bakape]

On second thought this might be a segfault turned infinite loop by the retarded signal handler interception GraphicsMagick is doing. So our first priority in this task should be to get rid of GraphicsMagick.

[Chiiruno]

I haven't had this happen since, so closing. If it is GraphicsMagick, I'll eventually get to that, seems like I'm starting to recover from being burnt out.