gh-ost icon indicating copy to clipboard operation
gh-ost copied to clipboard

Stopping infinite goroutines

Open nikhilmat opened this issue 7 years ago • 7 comments

Hey all!

Thanks for building such an awesome library!

We're getting ready to start using this as a part of our production workflow, and we wanted to build a wrapper around the gh-ost utility to manage some configuration on our end and to integrate it into our development/deployment workflows. We were hoping to build this wrapper in Go and consume the library directly.

When building out the proof of concept, we noticed that migrator.go and a few other places are starting infinite goroutines (for example the initiateStatus) that seem to be relying on the fact that it is invoked from a CLI and that the program will exit when Migrate() is complete. The behavior we saw was even after the migration completed, statuses were still being reporting to STDOUT.

The wrapper we are building will be a long running process (http server) that will manage the invocations of the gh-ost library. Would you be willing to accept a PR that cleans up these infinite goroutines, allowing the core logic to be used outside of the single CLI run?

Happy to hear any feedback on this approach if not. Thanks again!

nikhilmat avatar Aug 07 '17 20:08 nikhilmat

@nikhilmat thank you!. Indeed the tool was designed to run as standalone, but there's no reason why it should have to stay that way. I'm happy to receive PRs to change that, please go ahead.

I'm also curious about the wrapper service; will this be a schema deployment management tool, in the likeness of shift?

shlomi-noach avatar Aug 08 '17 04:08 shlomi-noach

I'm definitely interested in this service as well! Would love to be able to use gh-ost to manage table alterations but have the changes be versioned/deployed with a schema management tool.

Xopherus avatar Aug 10 '17 21:08 Xopherus

@Xopherus another thing we plan on looking at in the next quarter is https://github.com/skeema/skeema to see if we can use it as part of our schema management and changes.

tomkrouper avatar Aug 11 '17 04:08 tomkrouper

Sorry for the late follow up on this issue! Finally got around to finishing up the changes and submitting a PR, looking forward to hearing feedback.

As far as how we will be using this wrapper service, it won't be a full blown schema management tool on it's own. We are building something like shift internally, but only for executing migrations - we wanted to continue to drive the schema changes by code checked into the applications, rather than submitting them via UI. This wrapper service however will be used to integrate into a tool that we will use to kick off, monitor, and interact with migrations that are executed by gh-ost.

The piece wrapping gh-ost we had intended to simply orchestrate multiple gh-ost migrations, contain logic/permissions for accessing our databases, support CREATE/DROP tables with a similar API, and contain the throttling/hook logic that we will share across our application's migrations.

Happy to share more about what we've built once we have the proof of concept put together!

nikhilmat avatar Aug 28 '17 22:08 nikhilmat

@nikhilmat I'm cleaning up old issues. Were you able to implement any fixes for infinite goroutines?

timvaillancourt avatar Jul 15 '22 17:07 timvaillancourt

@timvaillancourt I believe @nikhilmat has implemented in https://github.com/github/gh-ost/pull/479.

By the way, I fixed some missing cleanups downstream https://github.com/bytebase/gh-ost/pull/3, would you like to have it merged on the upstream?

RainbowDashy avatar Jul 19 '22 09:07 RainbowDashy

By the way, I fixed some missing cleanups downstream https://github.com/bytebase/gh-ost/pull/3, would you like to have it merged on the upstream?

@RainbowDashy thanks! An upstream PR sounds useful 🙏

timvaillancourt avatar Jul 19 '22 11:07 timvaillancourt