Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have the following Lua script which I use against my redis environment to perform some non-production analytics on my data.

My dataset contains hundreds of millions of records, hence I want to make sure that the approach that I am taking is optimal for query performance and best coding practices.

-- Tables to store the aggregations
local aUsers = {}           -- [sizeInBytes] = userID      (Table is created the other way around to allow for sorting)
local aFileTypes = {}       -- [fileTypeID] = sizeInBytes
local aFileTag = {}         -- [fileTagID] = sizeInBytes

-- Global Count of Records and Sum of Sizes in Bytes
local sumCount = 0
local sumSize = 0

-- For users 'sizeBytes' is the Table[ID] to allow sorting. Collissions are not a big deal
local function userAgregations(table, key, size)
    table[size] = key
end

-- For other agregations the `fieldID` is the key mapped against the `size` agregation
local function genericAgregations(table, key, size)
    if table[key] then
        table[key] = table[key] + size
    else
        table[key] = size
    end
end

-- For users only, sort the table by Key (size) and print return it
local function printTopUsers(t)
    local str = "["
    local sortedTable = {}
    local maxResults = 25

    for key in pairs(t) do 
        sortedTable[#sortedTable+1] = key
    end

    table.sort(sortedTable, function(a,b)
        return a > b
    end)

    if #sortedTable < 25 then
        maxResults = #sortedTable
    end

    for i=1, maxResults do
        str = str .. sortedTable[i] .. ":" .. t[sortedTable[i]] .. ","
    end

    local formatted = str .. "]"
    return formatted
end

-- For other fields just return the table
local function printTable(t)
    local str = "["
    for key,value in pairs(t) do 
        str = str .. key .. ":" .. value .. ","
    end

    local formatted = str .. "]"
    return formatted
end

-- Return all the keys in redis that match a given pattern (Yes, its non-production)
-- Where the keys are:  0:0000:00:00000:00:00:00:0
-- Sample query is:     5:123?:??:12346:1?:??:??:2--
local matches = redis.call("KEYS", query)

-- For each Key match, get the value and perform the agregations
for _,key in ipairs(matches) do
    local val = redis.call("GET", key)
    local sCount, sSize = val:match("([^:]+):([^:]+)")  
    local count = tonumber(sCount)
    local size = tonumber(sSize)

    -- Extract a part of the key which refers to a particular agregation fieldID
    userAgregations(aUsers, string.sub(key, 11, 15), size)
    genericAgregations(aFileTypes, string.sub(key, 17, 18), size)
    genericAgregations(aFileTags, string.sub(key, 17, 18), size)

    -- Increment the Global sum and count
    sumCount = sumCount + count
    sumSize = sumSize + size
end

-- Contatenate and return response
local globalAgregation = string.format("[%s:%s]", sumCount, sumSize)
local result = printTopUsers(aUsers) .. printTable(aFileTypes) .. printTable(aFileTags) .. globalAgregation

return result
share|improve this question

2 Answers 2

up vote 5 down vote accepted

There are a few little things that can be improved:

-- For users 'sizeBytes' is the Table[ID] to allow sorting. Collissions are not a big deal
local function userAgregations(table, key, size)
    table[size] = key
end

If using LuaJIT, this function might be optimized away, but in regular Lua there's no reason to make a function that just does an assignment like this. Just do it straight in your code; table assignments are supposed to be understandable.

In genericAgregations (should be genericAggregations!) you're performing one table access too many:

local function genericAgregations(table, key, size)
    local value = table[key]
    table[key] = value and (value + size) or size
end

In printTopUsers, don't use a custom function in table.sort to sort in descending order. Sort normally and then perform the numeric for backwards.

Also, instead of concatenating str, create a temporary table, insert the bits of the string in it and in the end generate the string at once with table.concat. It's faster than multiple concatenations.

Same in printTable.

As for the redis-specific stuff, I have no knowledge but I hope the little tweaks above are useful!

share|improve this answer

Don't build strings via concatenation loops like this:

for key,value in pairs(t) do 
    str = str .. key .. ":" .. value .. ","
end

Since Lua strings are immutable and interned, this can end up being O(N^2). Instead, put the string parts in a table and do a table.concat to join everything in the end.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.