IGNITE-13033 Java thin client: Service invocation

[alex-plekhanov]

Thank you for submitting the pull request to the Apache Ignite.

In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:

The Contribution Checklist

• There is a single JIRA ticket related to the pull request.
• The web-link to the pull request is attached to the JIRA ticket.
• The JIRA ticket has the Patch Available state.
• The pull request body describes changes that have been made.
  The description explains WHAT and WHY was made instead of HOW.
• The pull request title is treated as the final commit message.
  The following pattern must be used: IGNITE-12407: Add Cluster API support to Java thin client
• A reviewer has been mentioned through the JIRA comments
  (see the Maintainers list)
• The pull request has been checked by the Teamcity Bot and
  the green visa attached to the JIRA ticket (see TC.Bot: Check PR)

Notes

• How to Contribute
• Coding abbreviation rules
• Coding Guidelines
• Apache Ignite Teamcity Bot

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

[ptupitsyn]

writeUuid supports null values - it writes an additional byte which is not needed here. See ClientClusterGroupGetNodeIdsResponse, for example: we usually write UUIDs as 8 bytes when null does not make sense.

[alex-plekhanov]

On the other hand, we have UUIDs with an extra byte in compute, partition awareness, and even handshake requests/responses. I think it should be somehow unified. At least with compute until it released (both should write nodeIds in the same format). WDYT?
Should we change compute as well? Or should we leave services as is?

[ptupitsyn]

I would prefer to avoid redundant data. Not because 1 byte matters for performance (in this case it does not), but because it makes things confusing for the client-side code: If there is a leading byte with uuid, it means that the value is potentially null. How should the client handle nulls here? Does not make sense.

Unfortunately, it is very tempting to use writeUuid, so these things keep showing up. Let's not change existing code, dealing with compat is hard.

[alex-plekhanov]

I've fixed it.
But it seems inconsistent now to use different formats for writing the same entity for compute and services.
Let's fix compute the same way (by another ticket of course)? It's not released yet, so there will be no compatibility issues.

[ptupitsyn]

Looks good to me 👍
Please see a couple minor comments.