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

Allow checking if Upgrader has a parent #24

Merged
merged 1 commit into from
Mar 29, 2019
Merged

Conversation

nolith
Copy link
Contributor

@nolith nolith commented Mar 28, 2019

There are situations in which is useful to detect the first invocation,
i.e. you may want to cleanup dangling unix sockets, but not during an
upgrade.

Closes #23

There are situations in which is useful to detect the first invocation,
i.e. you may want to cleanup dangling unix sockets, but not during an
upgrade.
@lmb
Copy link
Contributor

lmb commented Mar 28, 2019

Thank you for your contribution! Agree that HasParent makes sense.

Are you experiencing problems with dangling Unix sockets? The library should handle removing them if they are unused in the child.

@nolith
Copy link
Contributor Author

nolith commented Mar 28, 2019

Thanks @lmb, no I had no direct problem with the library.

What happened in my case is that the codebase I'm working on removes all the Unix sockets on boot before listening.

This is useful in case something went south on a previous run and you have the unix files left on disk.

With tableflip this approach will remove the unix socket during an upgrade and then even if upg.Fds.Listen("unix", path) returns a valid socket, you no longer have the file and clients can't connect.

As a first attempt I had to:

  • con, err = upg.Fds.Listen("unix", path)
  • if fails and file exist
    • delete file
    • if fail -> return nil, err
    • return upg.Fds.Listen("unix", path)
  • return con, err

But I think you will agree that this adds a lot of complexity for no good reason

@lmb
Copy link
Contributor

lmb commented Mar 29, 2019

Sure thing! I was contemplating whether this should be exposed, but didn't have a good use case in mind.

Thanks again for contributing, and for writing tests as well!

@lmb lmb merged commit 8392f16 into cloudflare:master Mar 29, 2019
@nolith nolith deleted the hasParent branch March 29, 2019 13:52
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.

2 participants