Mget binary support

[balajivenki]

@ipapapa and @shailesh33 I have implemented the mget with support for byte[] keys. Here are the summary of changes

• Implementing MultiKeyBinaryCommands interface and providing implementation for mget with byte[]
• Modified MultiKeyOperation to take binaryKey and assign it based on the first key in the List passed.
• The earlier assumption for String keys will not be null and now added null check
• CompressionValueMultiKeyOperation is modified to support changes from MultiKeyOperation
• I did a unit test with String mget and byte[] mget and the keys are span across multiple nodes. The result was correct.
• Now that we are implementing MultiKeyBinaryCommands this opens the door for byte[] implementation for other missing REDIS commands too and I will add them at later stage.

Please review the PR and let me know if anything needs to be added or modified.

[shailesh33]

Looks good to me

[shailesh33]

@ipapapa please review this PR

[ipapapa]

Is there a case that the firstKey is null and the rest are not?

[balajivenki]

@ipapapa good point. I have addd the null filter to address it and when all nulls sent it fails. Even the old implementation had this issue.

[balajivenki]

@ipapapa Even when we filter nulls the change for this is across all places where ever we use MultiKeyOperation and CompressionValueMultiKeyOperation and it will fail saying

redis.clients.jedis.exceptions.JedisDataException: value sent to redis cannot be null

so its a known behavior even with current implementation. If you want I can go ahead and add a Collections filter on all places before passing the keys down further. Please let me know.

[ipapapa]

Seems we are seeing the same issue in Travis as in #222

[balajivenki]

@ipapapa I think it is to do with nebula dependency. In my local I can see it can be fixed by upgrading nebula.netflixoss to 5.0.0

[ipapapa]

Can you update Nebula in this PR to fix it?

[balajivenki]

@ipapapa Yes upgrading to nebula 5.0.0 fixed the issue. I have removed the single line I added for filtering null as it is causing an exception. I will add it though, rest changes stays the same. Between is dyno jdk 8.0 supported? I see #185 is merged though. Please let me know. This is for using stream.filter.

[balajivenki]

Thanks a lot @ipapapa and @shailesh33

[ipapapa]

Would you like me to draft an RC?

[balajivenki]

Yes @ipapapa I was going to ask RC too.

[ipapapa]

Since #223 was fired at the same date. Let us wait to get this is approved as well and I will draft an RC tomorrow.

[balajivenki]

Thanks for the update @ipapapa

[ipapapa]

Done: https://github.com/Netflix/dyno/releases/tag/v1.6.4-rc.3

[balajivenki]

Thanks @ipapapa Still not available maven repo, waiting for it to be available there.

[balajivenki]

@ipapapa and @shailesh33 Can you please push the new rc into maven repo?

We only have till rc.2 there(https://mvnrepository.com/artifact/com.netflix.dyno/dyno-core)

[ipapapa]

It is there: https://bintray.com/netflixoss/oss-candidate/dyno/1.6.4-rc.3

[balajivenki]

Not sure why I am not able to see it in maven repo website though.