Skip to content

zqd: Prevent orphaned zqd (unix)#1031

Merged
alfred-landrum merged 2 commits intomasterfrom
orphaned-zqd
Sep 3, 2020
Merged

zqd: Prevent orphaned zqd (unix)#1031
alfred-landrum merged 2 commits intomasterfrom
orphaned-zqd

Conversation

@mattnibs
Copy link
Contributor

Prevent orphaned zqd when Brim exits with SIGKILL.

If platform is not windows, create posix pipe and pass read fd
to zqd via the -brimfd argument. Should brim be closed with SIGKILL
the fd will be closed and zqd, seeing the fd has closed will exit.

Closes #1018

@mattnibs mattnibs force-pushed the orphaned-zqd branch 2 times, most recently from 0a7bff8 to c6ebcb8 Compare August 27, 2020 19:38
@mattnibs mattnibs requested review from a team and alfred-landrum August 27, 2020 19:38
@mattnibs
Copy link
Contributor Author

zqd changes here: brimdata/super#1184

@mattnibs mattnibs force-pushed the orphaned-zqd branch 4 times, most recently from 1244bcd to 751e431 Compare August 28, 2020 23:50
Prevent orphaned zqd when Brim exits with SIGKILL.

If platform is not windows, create posix pipe and pass read fd
to zqd via the -brimfd argument. Should brim be closed with SIGKILL
the fd will be closed and zqd, seeing the fd has closed will exit.

Closes #1018
if (process.platform !== "win32") {
const {readfd} = require("node-pipe").pipeSync()
opts.stdio.push(readfd)
args.push(`-brimfd=${opts.stdio.length - 1}`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this works so I may be off, but it looks like you'll pass in the integer 3 here. Were you intending to access the array opts.stdio[opts.stdio.length - 1]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intended. See #1031 (comment)

@alfred-landrum alfred-landrum added this to the Brim v0.17.0 milestone Sep 2, 2020
@jameskerr jameskerr self-requested a review September 3, 2020 22:27
Copy link
Contributor

@jameskerr jameskerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works when I tested it out on my machine.

@alfred-landrum
Copy link
Contributor

Merging for @mattnibs who's out at the moment.

@alfred-landrum alfred-landrum merged commit 2d67442 into master Sep 3, 2020
@alfred-landrum alfred-landrum deleted the orphaned-zqd branch September 3, 2020 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Leaked zqd process on ungraceful brim shutdown

3 participants