appulse-projects/encon-java

ErlangPid lombok-generated hash code is inconsistent wrt PID types

tkroman opened this issue · 5 comments

Since there is an @EqualsAndHashCode(callSuper = true) annotation on ErlangPid, type field is considered for hashcode & equals, which means otherwise identical but different in PID/NEW_PID ErlangPids would be considered different, which means we don't get replies.
I bumped into this by simply launching elixir (iex --sname foo) and then doing ping from JVM.
After building locally with hashcode() defined as

@Override
  public int hashCode() {
    return Objects.hash(descriptor, id, serial, creation);
  }

message exchange works. I can submit a PR if this is the right solution or I would be happy to hear your thoughts on this in general.

another issue is that descriptor may be null by default but can be non-null in incoming pid, so proper equality should be done via getters, not fields.

Hi! First of all, thanks a lot for your issue report, the problem related not only to PIDs but for all terms, so I just set callSuper=false and explicitly force to use getters instead of fields for equals and hashCode.

The new version is already published and will be available soon by version number 1.6.10.

I didn't find your public contacts, so I'm going to ask here:

@tkroman , could you give me feedback about your Encon usage? What do you like or not in it? Do you use it "just for fun" or at your job? Or maybe you create something similar or something else?

I would like to make a new major release (with all new Erlang's Distribution Protocol features), but I'm not sure if someone really uses it. So, I will be happy if you share your thoughts about Encon with me.

Hi! Thanks for a quick reply! I was not sure if you would react so I forked this and released a fix under my own coordinates (with all the proper attributions to the original authors) - nice to know you keep track of the project so I could go back to using mainline releases.

I'm using encon (&epmd-java too actually) for a project I'm working on, it's based around interfacing with a lot of existing elixir services. Their authors didn't provide public APIs of any form so we are trying to teach our newer services communication with existing ones. It's been mostly good so far, API is nice, library is pretty much stable and covers majority of our usecases. So thanks again for such a quality product!

I've bumped into a couple of (non-major) issues so far:

  • we are using kotlin so databind doesn't work out of the box (which I solved by writing like 200 LOC for ErlangTerm encoding/decoding, so that's fine). Btw we are also using spring, but I also didn't go for spring module since basic library APIs cover every need.
  • a bigger issue for me is that there is no :rpc.call analog, which I'm working around by having a dedicated per-thread mailbox (so, let's say 4 mailboxes for my 4 worker threads) and doing sync/blocking send + receive. I'm not aware of a better way but so far seems OK too. If messages included request ids on protocol level (like erpc does), that would allow for async communication where we just create a promise that's resolved when a reply for a given requestId comes back but I guess that's not supported yet.

Other potential improvements would be Date/DateTime support (i'm doing manual decoding, it's not that hard) etc.

If you planned to work on this or any other protocol-level improvements, feel free to let me know and I'll see if I could lend a hand since I'm pretty much invested in this project now :)

I'm available for a chat on gitter if that's more convenient for you.

Thanks for your time and good words about my lib, it's really nice to hear! =)

About your issues:

  • I don't have exp in Kotlin at all. Could you provide some examples of your Kotlin code, which doesn't work with encon-databind (feel free to write me xxlabaza@gmail.com)?
  • Did you try mailbox.call and mailbox.receiveRemoteProcedureResult?
import io.appulse.encon.mailbox.Mailbox;
import io.appulse.encon.terms.ErlangTerm;
...
Mailbox mailbox = node.mailbox().build();
mailbox.call(myRemoteNode, "erlang", "date");
ErlangTerm response = mailbox.receiveRemoteProcedureResult(5, SECONDS);

This part of API is not obvious, but as I remember - I moved call and receiveProcedureResponse from Mailbox to Node in encon ver. 2, so you don't need to create mailbox for RPC explicitly and it looks more clear.

  • As I understand, there is no standard term structure for date/datetime representation, so I'm going to add support of all formats from qdate. It seems pretty popular and common approaches to represent the dates. Will it help to solve your needs?
  • on account of RPC - yes, that's what I'm doing right now. I meant that I'm doing blocking combination of call+receive wrt mailbox because if i decide to share a mailbox between threads, there is no guarantee that receive will return reply for the corresponding call. This would be solved by erpc-like protocol when we can send_request, get a couple of Refs and then track/resolve replies via receive_response (https://github.com/erlang/otp/blob/master/lib/kernel/src/erpc.erl#L175).

  • on account of dates - I guess I forgot this isn't supposed to be elixir-exclusive library, but Elixir does have established format for dates so maybe allowing for configuration on some level would be helpful.

  • on account of databind - there is a bunch of related issues, e.g.: kotlin's data classes are immutable and don't provide a default constructor no-arg and setters - this doesn't work well. This definitely can't be attributed as a shortcoming of this library, people usually solve this by having additional modules, e.g for jackson (setter-less instantiation via constructor): https://github.com/FasterXML/jackson-module-kotlin/blob/master/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt#L128