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

[12.0 stable] Prevent deferred queue from growing #4028

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

rouming
Copy link
Contributor

@rouming rouming commented Jul 1, 2024

Backport of the #4017 PR.

@eriknordmark
Copy link
Contributor

Please add reference to commit hash in master to your commit messages.

@rouming
Copy link
Contributor Author

rouming commented Jul 1, 2024

Please add reference to commit hash in master to your commit messages.

Will do once merged into master, otherwise SHA can be different.

…C reqs

LOC has different URL, so controller and LOC deferred requests can't be
placed into the same queue due to the same requests keys. Having two
destinations (controller + LOC), the second sender overrides a deferred
item, so first one destination will be never reached.

First attempt to fix this was made here:

   62b00e2 ("zedcloud/deferred: respect URL argument as a key on starting new deferred")

But this introduces more problems then fixes them (for example
`RemoveDeferred()` was not covered at all).

This is the proper way to deal with different destinations.

Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
(cherry picked from commit 713af84)
This fixes the growing deferred queue issue caused by missing
request merges. Commit 9f2c9ad ("zedcloud/deferred: don't
take lock while processing the deferred queue") relaxes locks
and makes it possible to add new requests while queue processing
waits in send. New requests should land in the same `deferredItems`
list, which should be processed later at the end of the
`handleDeferred()` operation. The bug lies in the actual merging
of two queues: the one that has not been completed due to a possible
error and the other queue, which was populated with new requests
during the send.

Proper queue merging is not just concatenation of queues (which
would cause the resulting queue to always grow), but involves item
replacement by key, so the queue size stays the same, with the item
being replaced.

Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Fixes: 9f2c9ad ("zedcloud/deferred: don't take lock while processing the deferred queue")
Reported-by: Milan Lenco <milan@zededa.com>
(cherry picked from commit 4582efb)
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

@rouming rouming merged commit ff855da into lf-edge:12.0-stable Jul 2, 2024
21 of 30 checks passed
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.

2 participants