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

Implement IPC with java 16 unix domain sockets #967

Closed
wants to merge 8 commits into from

Conversation

SpencerPark
Copy link

@SpencerPark SpencerPark commented Jan 21, 2024

Compatibility is a bit tricky because the current "ipc" implementation isn't ipc, so anyone relying on the jeromq tcp loopback implementation intentionally will break. I think this would only really be a problem if mixing an older and newer jeromq client and server. If both are updated then this should just start using unix domain sockets underneath with the same interface and the consumer wouldn't need to be aware of that difference.

Since this ipc implementation relies on a newer feature, I wanted to leave the existing tcp loopback implementation when running on a jvm that is missing the unix domain sockets. Thought a multi-release jar was the best approach for that targeting java 16 for the real ipc implementation.

To check at runtime if using the tcp loopback or unix domain socket implementation, you can query IpcAddress.isImplementedViaTcpLoopback().

The approach for the imlementation was to just pull all the non-tcp specific code out of TcpListener and TcpConnector into an abstract base class leaving abstract methods for the protocol specific details. The unix domain socket interface is still java.net.{ServerSocketChannel,SocketChannel} so a lot of that implementation can be shared.

Removed zmq.io.net.{ProtocolFamily,StandardProtocolFamily} android replacements because they are in 24 (looks like jeromq targets 28 currently?) and I needed to use the java.net. ones.

Fixes #953 and fixes #927

@SpencerPark
Copy link
Author

A multi-release jar seemed like the right approach for this but IDE support when targeting multiple versions in the same module isn't great. Happy to refactor if there are any suggestions for a better way to organize things. No issues building with maven on command line though.

I was able to successfully use this branch to bind a set of ipc server sockets that pyzmq can connect to. This is the motivation for implementing this feature (used for a jupyter kernel implementation on the jvm).

@fbacchella
Copy link
Contributor

I started a work on providing network protocols as services, but never had time to finish it. It should be a good starting point.

@SpencerPark
Copy link
Author

@fbacchella Referring to networkprotocolserviceprovider? Could you elaborate a bit on what you mean? There is still the problem of picking the ipc implementation at runtime and building part of jeromq targeting java 16 from what I can tell. i.e. if working off that branch I would still take the same approach here just in a different place.

I don't think I could pick up where you left off on that branch but I also don't think these changes necessarily conflict either. That work would still be useful regardless of this PR.

@fbacchella
Copy link
Contributor

It could be a simple jar dropped in the classpath. So no problem with java version and the need to use the module trick.

@SpencerPark
Copy link
Author

Thanks, ok so you'd prefer the new ipc implementation to be a separate artifact? Leave the existing one there and put both behind an SPI? This would be an extra maven dependency that consumers would need right?

@fbacchella
Copy link
Contributor

yes, that’s exactly what I had in mind.

@SpencerPark
Copy link
Author

SpencerPark commented Jan 28, 2024

@fbacchella Switched to loading IpcImpl via service loader. In order to add the extra artifact I had to move the current maven module into jeromq/ and the top level pom is now just there to serve as the parent for that and a new jeromq-ipc module.

The new module reruns the ipc tests I could find against the new implementation. The old fake ipc implementation is in zmq.io.net.ipc.loopback and it is used by default unless a service provider is found. I removed IpcAddressMask and the associated ipcAcceptFilters option because they were unused (at least in this code base) and I'm not really sure what it's supposed to be used for.

Please take a look when you get a chance.

@fbacchella
Copy link
Contributor

This PR is why I released 0.6.0 and launched 0.7.0-SNAPSHOT. Protocol will be provided as services. I need to investigate how to resolved the multi-release jar problem, but I think the way it’s handled in this PR is the good way.

@SpencerPark
Copy link
Author

@fbacchella I should update the description again because I got rid of the multi-release jar with the service oriented refactor but I'm kind of on the fence if it should be added again. i.e. still keeping the separate maven module and artifact but package that artifact as the multi-release jar, no idea how a service provider is declared with that packaging though.

For my use case I always want to use this new ipc implementation if running in jre >=16 and just not use ipc at all otherwise. So I would want to always include jeromq and jeromq-ipc on the classpath but ideally still work (just tcp) if running on java 11. Service provider with multi-release targeting 16 feels like the best option but please let me know your thoughts.

I also see you revived the protocol as a service branch (#974), I can try and rebase this PR based on that if the plan is to merge that PR as well. Please let me know and I can try and find some time this weekend :)

@fbacchella
Copy link
Contributor

The multi release jar is a good solution to provide a simple way to use IPC. Once the PR is merged, I will create another PR to implement modulorisation. Then we will be able to merge your implementation.

This was referenced Mar 17, 2024
@fbacchella
Copy link
Contributor

The new multi module organisation has just been merged. If you agree I will create a branch with empty IPC socket module to demonstrate how I think this should be handled. Or I can implement it myself, stealing the ideas, but you will miss the glory of attribution

@fbacchella
Copy link
Contributor

I have a working branch that could do the job that stole some of your ideas, see https://github.com/fbacchella/jeromq/tree/ipc_unix_socket.

@trevorbernard
Copy link
Member

This PR added unix domain socket support: #998

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.

ipc ipc:// protocol with zeromq. Java does support UNIX domain socket since JDK 16
3 participants