aerospike / aerospike-client-csharp

Aerospike C# Client Library
70 stars 48 forks source link

UDF Returns Wrong Value for Certain Longs #15

Closed Smoles closed 8 years ago

Smoles commented 8 years ago

Hello, I mentioned to Ronen on a call yesterday that we were seeing some weird behavior with a UDF vs a normal Get() operation and he suggested I post the bug here.

When storing certain long values in Aerospike we are seeing different results come back based on how we retrieve the data. If I insert the following value 717438855724142593 into a BIN and use the client.Get() method I get the same value back. However, if I get the value in a map from a UDF the value returned is 717438855724142592 (1 less than the original value) (Note that we've also seen other times where the value is off by 2).

Side note: We also tried this exact same thing using the PHP driver and it reproduced the same results.

Also note that we are using Aerospike 3.7.5.1 and the the C# Aerospike Client 3.1.7.

Below you will find the C# code and UDF I was using.

C# Code

using (AerospikeClient client = new AerospikeClient(new ClientPolicy(), new Host("YourHostName", 3000)))
{
    Key udidKey = new Key("yournamespace", null, "testkey");

    WritePolicy insertPolicy = new WritePolicy()
        {
            recordExistsAction = RecordExistsAction.UPDATE
        };

    client.Put(insertPolicy, udidKey, new Bin("Id", 717438855724142593));

    Record rec = client.Get(null, udidKey);

    IDictionary recordAsMap = (IDictionary)client.Execute(
        null,
        udidKey,
        "DeviceUdfs",
        "FetchForApp",
        Value.Get(new List<string>() {"Id"}),
        Value.Get(123),
        Value.Get("Publisher"));
}

UDF

-- asRecord is an Aerospike record.
-- binNames is a List (special type in Aerospike).
-- appId type is an int.
-- awrType is a string.
function FetchForApp(asRecord, binNames, appId, awrType)

    local results

    if not aerospike:exists(asRecord) then
        results = nil
    else
        --map is a special type provided by Aerospike.
        results = map()

        --Map all the BINs to the results map.
        for binName in list.iterator(binNames) do
            results[binName] = asRecord[binName]
        end

        results.Generation = record.gen(asRecord)

        if appId ~= nil then
            --Find the matching Last App Was Run record if it exists.
            local awrMap

            if awrType == 'Publisher' then
                awrMap = asRecord.LastPubAwr
            elseif awrType == 'Advertiser' then
                awrMap = asRecord.LastAdvAwr
            end

            if awrMap ~= nil then
                results.LastAwr = awrMap[appId]
            end
        end
    end

    return results
end
BrianNichols commented 8 years ago

One of the problems with Lua 5.1 is that all numbers are stored as doubles. Doubles allocate 52 bits for the mantissa. 717438855724142593 uses 60 bits of the possible 64 bits in a long. Therefore, precision will be lost when long values requiring more than 52 bits are converted to a double.

Lua 5.3 introduced a 64 bit integer type which would solve this conversion issue. The problem is Aerospike Server runs Lua 5.1. The server needs to upgrade to Lua 5.3.

bbulkow commented 8 years ago

Interestingly, Aerospike's internal Module architecture allows running multiple interpreters (or, in this case, multiple versions of an interpreter) side by side. Thus a second module calling the Lua 5.3 interpreter could ( fairly ) easily be created --- and thus remove the compatibility issues of code which requires 5.1 ( since 5.3 is not exactly a proper superset - memory use is higher, for example ).

A community contribution of adding the 5.3 Lua to the existing LuaJIT would be considered favorably.

Smoles commented 8 years ago

Would be nice if the client knew about this limitation and warned you about it. Maybe throw an exception, though that might be kind of ugly. At the very least you guys should update the documentation on your web site with a big giant warring about this limitation. (Maybe it's there already and I just over looked it).

Smoles commented 8 years ago

Looks like this limitation is documented. Though you have to dig a bit to find this info. http://www.aerospike.com/docs/udf/known_limitations.html