No clear way to get primary key in storage provider

[tsibelman]

Hi I have a custom storage provider and was trying to get the primary key from GrainReference that will work for all cases, this is a best solution that I was able to come to.

object rawKey = null;
string strKey = null;

try {
   rawKey = grainReference.GetPrimaryKeyLong(out strKey);
   if ((long)rawKey == 0) {
      rawKey = strKey;
   }
}
catch(InvalidOperationException){
   rawKey = grainReference.GetPrimaryKey(out strKey);
}

Why such a weird solution ? You may ask, I explain:

GrainReference has several methods that expose the information about primary key.

GetPrimaryKeyString
GetPrimaryKey
GetPrimaryKeyLong

GetPrimaryKeyString is internally calling overload of GetPrimaryKeyLong that has string output parameter. Because of this it will return string for grains with long compound keys and for grains that have only string key, so it not that useful and actually misleading. So I use GetPrimaryKeyLong by myself I saw that for grains with string keys GetPrimaryKeyLong will return zero as return param, so it actually not possible to distinguish between compound key that have long part set to zero and string key, I guess that is not that common for users to use zero for their key, but it still a bug.
In addition there no way other that getting an exception to check if primary key is long or guid.

I don't understand why GrainReference do not store the a type for it key, it would solve all of this weir mess.

[veikkoeeva]

@tsibelman This might be a more complete way to extract the key from the public API. At least Guid is considered too.

Also a thing here is that Guid.Empty is also 0 in the long key and it is not possible to distinquish between the two without an exception. I think I've read somewhere the reason for this has been historically that long has been considered to be one half of the Guid (though Guids are certain type of UUIDs and they have a specificiation, so I'm not sure if this holds in all cases).

[tsibelman]

@veikkoeeva Why in your solution if primaryKey returned from grainReference.GetPrimaryKeyString() is not null you disregard the fact that it can contain long or guid part ?

Internally grainreference have IsLong property exposing it will help somewhat but it will not solve all the issues.

[veikkoeeva]

Isn't it exclusively either string, long or Guid as defined by the grain interface docs? I looked at the interface and saw only the long and Guid an out parameter for the extension key.

Reading the documentation it looks like the intended, public usage has been within the grain where one already knows the key.

[tsibelman]

Key can be string, long, Guid, or one of the compound keys log and string, guid and string. So GetPrimaryKeyString will return you string for 2 different types of keys.

[veikkoeeva]

What two different types of key GetPrimaryKeyString returns? I didn't notice the string key would be non-null in any other case than when either long or Guid version was defined.

My logic is (I phrased poorly the previous passage) that strings IDs don't have an extension key. Then the order I check for the keys in that specific order is for the exact reason long and Guid version will always return something, see the exception and the problematic relation between long and Guid. Then only long and Guid versions can have a user defined extension.

Maybe in other words, I'm afraid you lost me, but I take another look later. :)

[tsibelman]

You have two keys:
compound key : long and string
string key

In my testing calling GetPrimaryKeyString will return you not null for both of these types. So you can't be sure if it one or the other key by just checking for null. I working with latest release but maybe it changed in the code after that

[tsibelman]

Here implementation of GetPrimaryKeyString from orleans\src\orleans\ids\grainid.cs
internal string GetPrimaryKeyString()
{
string key;
var tmp = GetPrimaryKey(out key);
return key;
}

[shayhatsor]

this is a duplicate of #1068.

[tsibelman]

@sergeybykov Do you have any thoughts about this issue ?

[sergeybykov]

@tsibelman As @shayhatsor mentioned in #1068, it looks like it would take a breaking change to solve this cleanly. I think it's a candidate for v2.0 where we'll likely have to make other major changes for .NET Core compatibility.

[sergeybykov]

Closing as a duplicate of #1068.