Skip to content

editoast: core_task: remove lock on valkey connection#17536

Open
flomonster wants to merge 2 commits into
devfrom
fam/fix-core-task-lock
Open

editoast: core_task: remove lock on valkey connection#17536
flomonster wants to merge 2 commits into
devfrom
fam/fix-core-task-lock

Conversation

@flomonster

Copy link
Copy Markdown
Member

While debuging our issue with valkey I found out that we lock the valkey connection with the new core_task framework.
The idea is to use the pool instead.

@flomonster flomonster requested a review from a team as a code owner July 7, 2026 09:39
@github-actions github-actions Bot added the area:editoast Work on Editoast Service label Jul 7, 2026
Comment thread editoast/core_task/src/lib.rs Outdated
Comment thread editoast/core_task/src/lib.rs Outdated
Comment thread editoast/cache/src/client.rs Outdated
@flomonster flomonster force-pushed the fam/fix-core-task-lock branch from 8774931 to b631194 Compare July 8, 2026 07:49
@flomonster flomonster requested a review from leovalais July 8, 2026 12:13
@flomonster flomonster force-pushed the fam/fix-core-task-lock branch from a0e6571 to cb123c6 Compare July 8, 2026 12:37
// Store the compressed values using mset
if !compressed_items.is_empty() {
span!(Level::INFO, "Sending items to Redis")
.in_scope(|| self.mset::<_, _, ()>(&compressed_items))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why doesn't this work?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

With @Castavo, we observed that the span duration was very short even if the query took way longer (even timed out). We see on the server side that the query takes ~1s. The issue is that we don't await the query response within the new span.

You think the previous version should work as I described? If yes, maybe there is something else going on here.

Comment thread editoast/core_task/src/lib.rs Outdated
);
}
Ok(serialized) => {
let vk_client = vk_client.clone();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: not sure this clone is necessary (resolve if I'm worng)

Signed-off-by: Florian Amsallem <florian.amsallem@gmail.com>
Signed-off-by: Florian Amsallem <florian.amsallem@gmail.com>
@flomonster flomonster force-pushed the fam/fix-core-task-lock branch from cb123c6 to 6264cac Compare July 9, 2026 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:editoast Work on Editoast Service

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants