[Out of curiosity] Can you please elaborate what problem this PR tries to solve?

I've been interested in something like this too, but I think I might prefer either just changing the function to only do full addresses or adding an extra argument to the function to enable it. This approach is certainly simpler than adding a new argument everywhere, but it also seems a little arbitrary to a user of the API.

[Out of curiosity] Can you please elaborate what problem this PR tries to solve?

We made this change unconditionally in fdb3, and our application now relies on the port number being present. The idea is to take advantage of the fact that \xff\xff keys never have a physical location to allow a caller to opt in to getting the port without breaking any existing callers

I think I might prefer either just changing the function to only do full addresses

I prefer this too, but I think we need to wait for an api version bump to change this behavior. This is intended as an undocumented backdoor that we can use until then.

This is intended as an undocumented backdoor that we can use until then.

The unfortunate thing about this is that I'm not sure we can ever really remove this once we add it. We'd have to at least continue supporting it for this API version, even if it we remove it in future ones.

To give some context on this: We introduced a change in our FDB 3 code to return ip:port instead of ip (as the IP alone is not very useful for this use-case) and forgot to port it to FDB 6. However, our throttling code relies heavily on that which means that we can't survive without it. Currently we're running with a patched version of the FDB-client but I would prefer having this in a next patch release (which is not difficult as it changes behavior).

I talked to Evan about this and he suggested to do this hack with the \xff\xff prefix. The main drawback of this hack, as @ajbeamon pointed out, is that we will have to support this in future versions. I am not sure whether this is a big problem. The plan would be to then return ip:port by default in FDB 7. We could deprecate the \xff\xff prefix potentially in FDB 7.1?

We also discussed adding a new API version for a patch version, but IMHO this is ugly as it breaks expectations that we are stable within releases. We could also add a new function, but this will be even more painful to maintain in the future.

So overall we are very sorry that we missed this :( But we will need some mechanism to support this.

I think my overall preference for this is to not use the \xff\xff approach, as it has the potential compatibility issues. Instead, using a database or transaction option (which could be deprecated in the future) or a knob (which I suppose could be made into a no-op in the future, at least for larger API versions) would not introduce any compatibility issues that I've thought of, and it also avoids having to change code to add the prefix and then remove it later.

EDIT: I just noticed your reply on the other comment chain after I submitted this.

It sounds like we're all (relatively) happy with using a database or transaction option, and then deprecating it in fdb 7? I'll get started on that.

Done. I figure I might as well add all documentation and testing properly, since other users may find the port useful and not want to wait for the next release. PTAL