Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow negative timeouts in all wait functions #7970

Open
Gerold103 opened this issue Nov 23, 2022 · 0 comments
Open

Allow negative timeouts in all wait functions #7970

Gerold103 opened this issue Nov 23, 2022 · 0 comments
Labels
app bug Something isn't working good first issue Good for newcomers

Comments

@Gerold103
Copy link
Collaborator

For example, netbox future:wait_result() fails on a negative timeout with an exception instead of just returning nil,err. That is quite unhandy. Consider this example (from vshard):

    for uuid, future in pairs(futures) do
        res, err = future:wait_result(timeout)
        -- Handle netbox error first.
        if res == nil then
            err_uuid = uuid
            goto fail
        end
        -- Ref returns nil,err or bucket count.
        res, err = res[1], res[2]
        if res == nil then
            err_uuid = uuid
            goto fail
        end
        bucket_count = bucket_count + res
        timeout = deadline - fiber_clock()
    end

Here I don't want to care whether timeout is 0 or -0.000001 (in this case I just missed it). I don't want to check it explicitly. Code is 5 lines shorter in this case when I can trust :wait_result() to gracefully return an error on a negative timeout. Otherwise I would need to add:

if timeout < 0 then
    err = box.error.new(box.error.TIMEOUT)
    err_uuid = uuid
    goto fail
end

(I will need to add it now anyway, because I have to support old versions too.)

The problem grows when I have many :wait_result() usages and have to invent one another vshard.util function to make it easier to use.

AFAIR the same problem is at least with our condition variables.

It might even be called inconsistent, because we legally allow to have negative timeouts - clock_* functions in clock.lua return int64_t specifically not to care about accidentally going below zero.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant