Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper for more complicated shutdown dependencies #3200

Closed
bartekn opened this issue Nov 9, 2020 · 2 comments
Closed

Helper for more complicated shutdown dependencies #3200

bartekn opened this issue Nov 9, 2020 · 2 comments
Assignees
Labels
horizon ingest New ingestion system

Comments

@bartekn
Copy link
Contributor

bartekn commented Nov 9, 2020

@tamirms:

I wonder if we can simplify the shutdown logic further by using contexts and passing them down to each component down to the stellarCoreRunner instance. Within stellarCoreRunner we can use https://golang.org/pkg/os/exec/#CommandContext to execute stellar core. Perhaps we can make an issue separate from this PR to investigate if using contexts would help.

Originally posted by @tamirms in #3187 (comment)

@bartekn

I looked into using CommandContext and I think it won't help much. We still need close() method to do some cleanup and we can't do it with context alone. But we can definitely investigate how to write a shutdown code for more complicated object connections. For example, to figure out how to close CaptiveCore, bufferedLedgerMetaReader and stellarCoreRunner I drew a graph to understand the dependencies.

To elaborate, I think a helper that maintains a dependency graph of services that can be shutdown (and in which order) will be helpful in Horizon.

@bartekn
Copy link
Contributor Author

bartekn commented Dec 1, 2020

I checked if any of existing packages implement something like this and I came across tomb. It's much simpler that what I was thinking about, for example no dependency graph but maybe it's actually better - we can create a graph implicitly (by using tomb within struct that starts go routines) rather than explicitly (by creating a graph struct with all dependencies).

I will try to create a quick experiment in CaptiveStellarCore to check if we can use it.

@bartekn
Copy link
Contributor Author

bartekn commented Dec 17, 2020

We just merged another approach here: #3278. Closing for now, will reopen if/when we want to revise the solution.

@bartekn bartekn closed this as completed Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
horizon ingest New ingestion system
Projects
None yet
Development

No branches or pull requests

1 participant