Move cron scheduling inside application #347
Conversation
* Move cron scheduling inside application * Make envvar a fallback and check for errors * Panic significantly less * propagate error out of runBackup * Add structured logging * FIx error propagation to exit * Enable the new scheduler by default * Review fixes * Added docs and better error propagation
|
@pixxon Just to give un update on the status of this: I did some minor tweaking in regards to error handling and logging and would think this is now ready to go from a code perspective. As we're now having a long running process though, I'd still like to confirm this doesn't leak memory somehow, so I'll let the current state of this branch ( |
|
Sounds great! I am not familiar with golang, but maybe there are some tools to help finding memory leaks. ( In the c++ word, there are some great static analyzers that help, or even better are the memory sanitizers that check for addresses that were not freed properly. ) |
|
The only way to really leak memory in Go is to have goroutines running that never return, meaning everything in their scope isn't possible to be garbage collected. I definitely managed to do this before, even when having static analysis look for such problems. I don't think it's likely here, but I'd feel safer if I've seen it running for a bit. One simple thing we could do is print the numbers of Go routines from time to time to see if the number stays stable, but I guess I'll defer that until it looks like there really is a leak. |
|
I got nerd sniped and fired up After doing this, the number of Goroutines seems stable when I run a per-minute schedule locally for ~15mins or so. See 68ba6ce as a fix I'll push this as |
|
Seems the advice on closing it is right there in the docs https://pkg.go.dev/github.com/docker/docker/client#section-readme .... |
* Hoist control for exiting script a level up * Do not accidentally nil out errors * Log when running schedule * Remove duplicate log line * Warn on cron schedule that will never run
|
I added some additional profiling options and had this running for a few days. So far, it doesn't look like anything is leaking to me, so I'll go ahead and merge and release this now. |
Closes #268