feat(explorer): local indexer inside explorer #3229
Conversation
🦋 Changeset detectedLatest commit: 6dee218 The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
karooolis
changed the title
karooolis
force-pushed
the
kumpis/indexer-inside-explorer
branch
from
312373d to
aa55ef9
Compare
karooolis
force-pushed
the
kumpis/indexer-inside-explorer
branch
from
d538550 to
21659a9
Compare
| @@ -143,6 +171,9 @@ process.on("SIGINT", () => { | |||
| if (explorerProcess) { | |||
| explorerProcess.kill(); | |||
| } | |||
| if (indexerProcess) { | |||
| indexerProcess.kill(); | |||
| } | |||
| process.exit(); | |||
There was a problem hiding this comment.
will this kill the subprocesses automatically?
There was a problem hiding this comment.
it does not, does indexer spawn subprocesses? wonder if we should use something like https://www.npmjs.com/package/terminate to stay on the safe side?
There was a problem hiding this comment.
By subprocesses I meant the things you're spawning here. I expect since this explorer bin process is the one spawning, it has a parent-child relationship, and would kill any of its children if the parent process got killed. But maybe that assumption is why a lot of my tools end up with hanging/zombie processes 🙈
Down to use something like terminate, but usage seems low, so I wonder if there's a more mainstream/battle-tested variant out there.
There was a problem hiding this comment.
Heh, yeah terminate package does not seem widely used, and might be an overkill. Just looked into it a bit more and based on my understanding, keeping track of child processes, and then killing them once the parent process is killed is the way to do it. A common danger is spawning child processes but then not killing them explicitly once parent process is killed but we handle that with process.on("SIGINT") listener so should be good there. Then the child processes, once stopped, should themselves stop their own child processes but looks like we have no problems there since both explorer and store-indexer don't seem to leave anything hanging.
There was a problem hiding this comment.
what about other signals like sigkill?
There was a problem hiding this comment.
Damn, thought I had everything. So turns out SIGKILL cannot be captured but SIGTERM can. So now capturing SIGINT and SIGTERM which captures it all to the best of my knowledge. Will do a bit more research into this
There was a problem hiding this comment.
doesn't help with sigkill but looks like we can listen to both sigint and sigterm with
process.on('exit', () => {
...
})There was a problem hiding this comment.
Just tried it and it's working. Checked the docs and it says The 'exit' event is emitted when the Node.js process is about to exit as a result of either: so it does make sense to clean up the child processes here based on the documentation, updated.
Co-authored-by: Kevin Ingersoll <[email protected]>
Co-authored-by: Kevin Ingersoll <[email protected]>
Local store indexer is now run as part of the Explorer package. It is skipped if the configured chain is non-anvil.