expirationd icon indicating copy to clipboard operation
expirationd copied to clipboard

Add ability to pass args to start_key function

Open ArtDu opened this issue 3 years ago • 2 comments

https://github.com/tarantool/expirationd/blob/c3b86c8bea7b7f6ddbf5b079836c60f3ee3976c1/expirationd.lua#L691-L698 https://github.com/tarantool/expirationd/blob/c3b86c8bea7b7f6ddbf5b079836c60f3ee3976c1/expirationd.lua#L382-L390

Should be rewritten to:

-- check start_key 
 if options.start_key ~= nil or options.start_key == box.NULL then 
     if type(options.start_key) == "function" then 
         task.start_key = function(...) return options.start_key(...) end 
     else 
         task.start_key = function() return options.start_key end 
     end 
 end 
-- default iterate_with function
local function default_iterate_with(task)
    return task.index:pairs(task.start_key(task.args), { iterator = task.iterator_type })
       :take_while(
            function()
                return task:process_while()
            end
        )
end

Or should we pass task as it says in README? https://github.com/tarantool/expirationd/blob/c3b86c8bea7b7f6ddbf5b079836c60f3ee3976c1/README.md#L94-L129

ArtDu avatar Jun 21 '22 19:06 ArtDu

Thank you for the issue!

It seems to me that we should to fix the README. I don't see a real case of usage the task argument in the start_key function considering that we only provide to an user the interface:

task.start (self)
task.stop (self)
task.restart (self)
task.kill (self)
task.statistics (self)

and the user should not use other fields or methods:

https://github.com/tarantool/expirationd/blob/c3b86c8bea7b7f6ddbf5b079836c60f3ee3976c1/expirationd.lua#L210-L213

oleg-jukovec avatar Jun 24 '22 11:06 oleg-jukovec

Thank you for the issue!

It seems to me that we should to fix the README. I don't see a real case of usage the task argument in the start_key function considering that we only provide to an user the interface:

task.start (self)
task.stop (self)
task.restart (self)
task.kill (self)
task.statistics (self)

and the user should not use other fields or methods:

https://github.com/tarantool/expirationd/blob/c3b86c8bea7b7f6ddbf5b079836c60f3ee3976c1/expirationd.lua#L210-L213

We can pass args instead task. It's useful when you want to start iteration from point and you depends on user data(args). For example, if I wanted to remove by time with some threshold I would use start_key

function get_lower_bound(args)
    return fiber.time() - args.threshold
end

expirationd.start(name, space, is_expired, {
  index = 'created_at_index',
  start_key = get_lower_bound,
  iterator_type = "LT"
  args = {
    threshold = 30 -- seconds
  }
})

And we've already had the flexibility of user callbacks which have task as argument https://github.com/tarantool/expirationd/blob/977c8cc3343f7ff6be162f8fb1fba799bdfe7a94/expirationd/init.lua#L347-L358

That's why I prefer to pass task for having consistent with others callbacks

ArtDu avatar Apr 05 '23 11:04 ArtDu