Count tuples from cache fetch requests in statistics

Don't count tuples from the cache as fetched ones. These changes made
the statistics more fair.

Return an iterator from a cache lookup call to make the API closer to
the regular data fetch request (accessor:get_index(...):pairs(...)).

Perform cache lookup from accessor_general, not from accessor_shard (it
allows to distinguish such requests and simplify implementation of the
accessor functions).

Author: Totktonada

graphql/accessor_general.lua

@@ -853,9 +853,8 @@ local function process_tuple(self, state, tuple, opts)
     local do_filter = opts.do_filter
     local pivot_filter = opts.pivot_filter
     local qcontext = state.qcontext
-    local qstats = qcontext.statistics
 
-    qstats:objects_fetched({
+    qcontext.statistics:objects_fetched({
         is_full_scan = opts.is_full_scan,
         objects_count = opts.fetched_tuples_cnt,
     })
@@ -881,11 +880,9 @@ local function process_tuple(self, state, tuple, opts)
         return true -- skip pivot item too
     end
 
-    -- Don't count subrequest resulting objects (needed for filtering) into
-    -- count of object we show to an user as a result.
-    -- XXX: It is better to have an option to control whether selected objects
-    -- will be counted as resulting ones.
-    local saved_resulting_object_cnt = qstats.resulting_object_cnt
+    -- XXX: implement retired flag for invoke resolve and don't save-restore
+    -- resulting_object_cnt
+    local saved_resulting_object_cnt = qcontext.statistics.resulting_object_cnt
 
     -- make subrequests if needed
     for k, v in pairs(filter) do
@@ -900,7 +897,7 @@ local function process_tuple(self, state, tuple, opts)
         end
     end
 
-    qstats.resulting_object_cnt = saved_resulting_object_cnt
+    qcontext.statistics.resulting_object_cnt = saved_resulting_object_cnt
 
     -- filter out non-matching objects
     local match = utils.is_subtable(obj, filter) and
@@ -916,7 +913,7 @@ local function process_tuple(self, state, tuple, opts)
     state.objs[#state.objs + 1] = obj
     state.count = state.count + 1
 
-    qstats:objects_retired({
+    qcontext.statistics:objects_retired({
         objects_count = 1,
     })
 
@@ -1138,18 +1135,30 @@ local function invoke_select_internal(self, prepared_select)
     local request_opts = prepared_select.request_opts
     local select_state = prepared_select.select_state
     local select_opts = prepared_select.select_opts
+    local collection_name = prepared_select.collection_name
     local args = prepared_select.args
     local extra = prepared_select.extra
 
     local index = request_opts.index
+    local index_name = request_opts.index_name
     local index_value = request_opts.index_value
     local iterator_opts = request_opts.iterator_opts
     local is_full_scan = request_opts.is_full_scan
 
     local tuple_count = 0
     local out = {}
 
-    for _, tuple in index:pairs(index_value, iterator_opts, out) do
+    -- lookup for needed data in the cache if it is supported
+    local iterable
+    if self:cache_is_supported() then
+        iterable = self.funcs.cache_lookup(self, collection_name, index_name,
+            index_value, iterator_opts)
+    end
+    if iterable == nil then
+        iterable = index
+    end
+
+    for _, tuple in iterable:pairs(index_value, iterator_opts, out) do
         local fetched_tuples_cnt = out.fetched_tuples_cnt
         check(out.fetched_tuples_cnt, 'out.fetched_tuples_cnt', 'number')
         out.fetched_tuples_cnt = 0
@@ -1644,11 +1653,32 @@ function accessor_general.new(opts, funcs)
             cache_is_supported = function(self)
                 return self.data_cache ~= nil
             end,
-            cache_fetch = function(self, batches)
+            cache_fetch = function(self, batches, qcontext)
                 if not self:cache_is_supported() then
                     return nil
                 end
-                local fetch_id = self.funcs.cache_fetch(self, batches)
+
+                local res = self.funcs.cache_fetch(self, batches)
+                if res == nil then
+                    return nil
+                end
+
+                local fetch_id = res.fetch_id
+                local stats = res.stats
+                check(fetch_id, 'fetch_id', 'number')
+                check(stats, 'stats', 'table')
+
+                -- update statistics
+                init_qcontext(self, qcontext)
+                for _, stat in ipairs(stats) do
+                    qcontext.statistics:objects_fetched({
+                        is_full_scan = stat.is_full_scan,
+                        objects_count = stat.fetched_tuples_cnt,
+                    })
+                end
+
+                -- XXX: check timeout
+
                 return fetch_id
             end,
             -- cache_delete = function(self, fetch_id)

graphql/accessor_shard.lua

@@ -179,29 +179,23 @@ local function get_index(self, collection_name, index_name)
     if not is_index_exists(collection_name, index_name) then
         return nil
     end
-    local accessor = self
-    local cache_lookup = self.funcs.cache_lookup
-    check(cache_lookup, 'cache_lookup', 'function')
 
+    -- XXX: wrap all data into the table, don't create the capture
     local index = setmetatable({}, {
         __index = {
-            pairs = function(self, value, opts, out)
+            pairs = function(_, value, opts, out)
                 out.fetched_tuples_cnt = 0
                 local func_name = 'accessor_shard.get_index.<index>.pairs'
-                -- check cache
-                local tuples = cache_lookup(accessor, collection_name,
-                    index_name, value, opts)
+
                 -- perform select
-                if tuples == nil then
-                    local opts = opts or {}
-                    opts.limit = opts.limit or LIMIT
-                    local err
-                    tuples, err = shard:secondary_select(collection_name,
-                        index_name, opts, value, 0)
-                    accessor_shard_helpers.shard_check_error(func_name,
-                        tuples, err)
-                end
+                local opts = opts or {}
+                opts.limit = opts.limit or LIMIT
+                local tuples, err = shard:secondary_select(collection_name,
+                    index_name, opts, value, 0)
+                accessor_shard_helpers.shard_check_error(func_name,
+                    tuples, err)
                 out.fetched_tuples_cnt = out.fetched_tuples_cnt + #tuples
+
                 -- create iterator
                 local cur = 1
                 local function gen()
@@ -210,6 +204,7 @@ local function get_index(self, collection_name, index_name)
                     cur = cur + 1
                     return cur, res
                 end
+
                 return gen, nil, nil
             end
         }
@@ -488,7 +483,18 @@ end
 ---
 --- @tparam table batches see @{accessor_shard_cache.cache_fetch}
 ---
---- @treturn number `fetch_id` identifier of the fetched data
+--- @treturn table the following structure or nil:
+---
+---     {
+---         fetch_id = <number>, -- identifier of the fetched data
+---         stats = {            -- data to update statistics
+---             {
+---                 is_full_scan = <boolean>,
+---                 fetched_tuples_cnt = <number>,
+---             },
+---             ...
+---         }
+---     }
 local function cache_fetch(self, batches)
     return self.data_cache:fetch(batches)
 end
@@ -525,7 +531,7 @@ end
 ---
 --- @tparam table iterator_opts e.g. {} or {iterator = 'GT'}
 ---
---- @treturn table `result` fetched data (can be nil)
+--- @return luafun iterator (one value) to fetched data or nil
 local function cache_lookup(self, collection_name, index_name,
         key, iterator_opts)
     return self.data_cache:lookup(collection_name, index_name, key,

graphql/accessor_shard_cache.lua

@@ -3,6 +3,8 @@ local shard = require('shard')
 local utils = require('graphql.utils')
 local accessor_shard_index_info = require('graphql.accessor_shard_index_info')
 
+local check = utils.check
+
 local accessor_shard_cache = {}
 
 -- XXX: maybe use a space instead of a Lua table
@@ -48,12 +50,13 @@ local function cache_fetch(self, batches)
     local fetch_id = self.fetch_id_next
     self.fetch_id_next = self.fetch_id_next + 1
 
+    local stats = {}
+
     for _, batch in pairs(batches) do
         local ids = get_select_ids(self, batch, {skip_cached = true})
         if next(ids) == nil then
             goto continue
         end
-        -- print('fetch ids: ' .. require('yaml').encode(ids))
         -- local buffers = {}
         local results = {}
 
@@ -79,6 +82,10 @@ local function cache_fetch(self, batches)
                 for _, tuple in ipairs(node_result) do
                     table.insert(results[j], tuple)
                 end
+                stats[j] = stats[j] or {
+                    is_full_scan = batch.keys[j] == nil,
+                    fetched_tuples_cnt = 0,
+                }
             end
         end
 
@@ -87,8 +94,9 @@ local function cache_fetch(self, batches)
         -- sort by a primary key
         local primary_index_info = accessor_shard_index_info.get_index_info(
             batch.collection_name, 0)
-        for _, result in ipairs(results) do
-            table.sort(result, function(a ,b)
+        for j, result in ipairs(results) do
+            stats[j].fetched_tuples_cnt = stats[j].fetched_tuples_cnt + #result
+            table.sort(result, function(a, b)
                 for i, part in pairs(primary_index_info.parts) do
                     if a[part.fieldno] ~= b[part.fieldno] then
                         return a[part.fieldno] < b[part.fieldno]
@@ -99,13 +107,10 @@ local function cache_fetch(self, batches)
         end
 
         -- write to cache
-        --print('#results: ' .. tostring(#results))
-        --print('#batch.keys: ' .. tostring(#batch.keys))
         assert(#results == #batch.keys, 'XXX') -- XXX
         assert(#results == #ids, 'XXX') -- XXX
         for i = 1, #ids do
             if self.cache[ids[i]] == nil then
-                -- print('fetch: ' .. require('json').encode(ids[i]))
                 self.cache[ids[i]] = {
                     result = results[i],
                     fetch_ids = {fetch_id}
@@ -122,7 +127,10 @@ local function cache_fetch(self, batches)
         ::continue::
     end
 
-    return fetch_id
+    return {
+        fetch_id = fetch_id,
+        stats = stats,
+    }
 end
 
 -- Unused for now.
@@ -169,12 +177,38 @@ local function cache_lookup(self, collection_name, index_name, key,
         iterator_opts = iterator_opts,
     }
     local id = get_select_ids(self, batch)[1]
-    local result
-    if self.cache[id] ~= nil then
-        result = self.cache[id].result
+    if self.cache[id] == nil then
+        return nil
     end
 
-    return result
+    local tuples = self.cache[id].result
+    check(tuples, 'tuples', 'table')
+    -- XXX: wrap all data into the table, don't create the capture
+    return setmetatable({tuples = tuples}, {
+        __index = {
+            pairs = function(self, value, opts, out)
+                assert(value == key, 'expected the same key in ' ..
+                    'cache_lookup call and :pairs() on returned iterable')
+                assert(opts == iterator_opts, 'expected the same ' ..
+                    'iterator_opts in cache_lookup call and :pairs() on ' ..
+                    'returned iterable')
+
+                -- we actually do not fetch any tuples here
+                out.fetched_tuples_cnt = 0
+
+                -- create iterator
+                local cur = 1
+                local function gen()
+                    if cur > #self.tuples then return nil end
+                    local res = tuples[cur]
+                    cur = cur + 1
+                    return cur, res
+                end
+
+                return gen, nil, nil
+            end
+        }
+    })
 end
 
 function accessor_shard_cache.new()