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

tube: fix slow take on busy utubes #229

Merged
merged 2 commits into from
May 18, 2024

Conversation

DerekBum
Copy link
Contributor

@DerekBum DerekBum commented Apr 28, 2024

If some of the utubes for tasks at the start of the queue were busy most of the time, take would slow down for every other task. This problem is fixed by creating a new space space_ready. It contains first task with READY status from each utube.

This solution shows great results for the stated problem, but with the cost of slowing the put method. Thus, this workaround is disabled by default. To enable it, user should set the storage_mode = "ready_buffer" as an option while creating the tube (or queue.driver.utube.STORAGE_MODE_READY_BUFFER for utube and queue.driver.utubettl.STORAGE_MODE_READY_BUFFER for utubettl). As example:

local test_queue = queue.create_tube('test_queue', 'utube',
        {temporary = true, storage_mode = queue.driver.utube.STORAGE_MODE_READY_BUFFER})

Or

local test_queue = queue.create_tube('test_queue', 'utubettl',
        {temporary = true, storage_mode = queue.driver.utubettl.STORAGE_MODE_READY_BUFFER})

Also added two benchmarks to compare storage_mode = "default" and storage_mode = "ready_buffer" (storage_mode = "default" is default and disables the workaround).

  1. Benchmark for simple put and take methods. 30k utubes are created with single task each. Task creation time is calculated. After that 30k consumers are calling take + ack, each in the separate fiber. Time to ack all tasks is calculated. The results are as follows (for comparison I also used fifo tube):
put (30k) take+ack
default 180ms 1.6s
ready 270ms 1.7s

As we can see, new utube implementation has slower put method.
2. Benchmark for the stated problem. 10 tubes are created. Each contains 1000 task. After that 10 consumers are created (each works on his tube only, one tube -- one consumer). Each consumer will take, then yield and then ack every task from their utube (1000 tasks each).
After that we can also run this benchmark with 10k tasks on each utube, 100k tasks and 150k tasks. But all that with 10 utubes and 10 consumers. The results are as follows:

1k 10k 50k 150k
default 53s 1.5h 100h 1000h
ready 450ms 4.7s 26s 72s

We can see great time performance improvement.

Same change was also made for utubettl. Here is benchmarks results for utubettl (on the same benchmarks):

put (30k) take+ack
default 200ms 1.7s
ready 320ms 1.8s
1k 10k 50k 140k
default 80s 1.6h 100h 1000h
ready 520ms 5.4s 28s 83s

(note that for busy tasks were used 140k and not 150k tasks).

Closes #228

@DerekBum DerekBum force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch from 9e7bd98 to 7827b24 Compare April 28, 2024 20:42
@DerekBum
Copy link
Contributor Author

Right now only utube is fixed. utubettl will follow soon.

Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add update the ready space for all updates of the READY state for a task cases: release, as example.

queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
@DerekBum DerekBum force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch 2 times, most recently from 86a4164 to 8b14921 Compare April 28, 2024 23:53
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
@DerekBum DerekBum force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch 5 times, most recently from 3afb16e to 80f33dc Compare May 9, 2024 12:23
@DerekBum
Copy link
Contributor Author

DerekBum commented May 9, 2024

Added fix for utubettl in a different commit.

Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, update the README.md.
We need notes about the utube/utubettl configuration, policy/option (I'm still not sure about the naming) differences and performance.

Please, apply fixes for the comments to both queues.

queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
@DerekBum DerekBum force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch 3 times, most recently from 0e0a63a to 3e07f7d Compare May 13, 2024 10:50
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
@oleg-jukovec
Copy link
Collaborator

oleg-jukovec commented May 13, 2024

@DifferentialOrange @better0fdead this is a non-trivial fix, we need additional reviews with a clear head here.

@DerekBum
Copy link
Contributor Author

Updated the code according to comments.
Added links to the new issue #230 about implementing new storage modes for vinyl engine.

README.md Show resolved Hide resolved
t/benchmark/busy_utubes.lua Outdated Show resolved Hide resolved
@DerekBum DerekBum force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch from 9d8360b to f78cb5d Compare May 14, 2024 17:10
@oleg-jukovec oleg-jukovec removed the request for review from better0fdead May 15, 2024 10:24
@DerekBum DerekBum force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch from f78cb5d to 1e1645c Compare May 15, 2024 11:01
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea seems rather clear after inspecting the issue and, since tests are green, everything should be fine. I had left some code nits, but it shouldn't be anything crucial there.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract.lua Show resolved Hide resolved
@DerekBum DerekBum force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch from 1e1645c to 7961003 Compare May 16, 2024 19:14
@DerekBum
Copy link
Contributor Author

Updated the code according to the comments.

Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the utubettl we don't take into account priority when working with STORAGE_MODE_READY_BUFFER.

@DerekBum DerekBum force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch 3 times, most recently from c9f6e74 to 9e58cf6 Compare May 16, 2024 21:01
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utubettl.lua Show resolved Hide resolved
queue/abstract/driver/utubettl.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utubettl.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
@DerekBum DerekBum force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch from 9e58cf6 to 9267b95 Compare May 17, 2024 11:52
@DerekBum
Copy link
Contributor Author

Updated the code according to comments.

@DerekBum DerekBum force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch 5 times, most recently from a7df866 to 31adb7d Compare May 17, 2024 17:06
queue/abstract/driver/utube.lua Outdated Show resolved Hide resolved
queue/abstract/driver/utubettl.lua Outdated Show resolved Hide resolved
If some of the utube for tasks at the top of the queue were busy
most of the time, `take` would slow down for every other task.
This problem is fixed by creating a new space `space_ready_buffer`. It
contains first task with `READY` status from each utube.

This solution shows great results for the stated problem, with the cost
of slowing the `put` method. Thus, this workaround is disabled by default.
To enable it, user should set the
`storage_mode = queue.driver.utube.STORAGE_MODE_READY_BUFFER` as an option
while creating the tube. As example:
```lua
local test_queue = queue.create_tube('test_queue', 'utube',
        {temporary = true, storage_mode = queue.driver.utube.STORAGE_MODE_READY_BUFFER})
```

Part of #228
If some of the utubettl for tasks at the top of the queue were busy
most of the time, `take` would slow down for every other task.
This problem is fixed by creating a new space `space_ready_buffer`. It
contains first task with `READY` status from each utube.

This solution shows great results for the stated problem, with the cost
of slowing the `put` method. Thus, this workaround is disabled by default.
To enable it, user should set the
`storage_mode = queue.driver.utubettl.STORAGE_MODE_READY_BUFFER` as an option
while creating the tube. As example:
```lua
local test_queue = queue.create_tube('test_queue', 'utubettl',
        {temporary = true, storage_mode = queue.driver.utubettl.STORAGE_MODE_READY_BUFFER})
```

Closes #228
@oleg-jukovec oleg-jukovec force-pushed the DerekBum/gh-228-optimize-utube-slow-take branch from 31adb7d to 64033a6 Compare May 18, 2024 18:45
@oleg-jukovec oleg-jukovec merged commit 8450859 into master May 18, 2024
23 checks passed
@oleg-jukovec oleg-jukovec deleted the DerekBum/gh-228-optimize-utube-slow-take branch May 18, 2024 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

utubettl: slow take
3 participants