Skip to content

Conversation

@dranikpg
Copy link
Contributor

@dranikpg dranikpg commented Jan 3, 2026

Related to #6314

Copilot AI review requested due to automatic review settings January 7, 2026 07:22

This comment was marked as outdated.

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
Copilot AI review requested due to automatic review settings January 8, 2026 19:54

This comment was marked as outdated.

@dragonflydb dragonflydb deleted a comment from Copilot AI Jan 9, 2026
@dragonflydb dragonflydb deleted a comment from Copilot AI Jan 9, 2026
@dragonflydb dragonflydb deleted a comment from Copilot AI Jan 9, 2026
@dragonflydb dragonflydb deleted a comment from Copilot AI Jan 9, 2026
@dragonflydb dragonflydb deleted a comment from Copilot AI Jan 9, 2026
Comment on lines 1381 to 1382
boost::intrusive_ptr<Transaction> keepalive{cmnd_cntx->tx};
auto replier = [mget_results, mget_resp, cmnd_cntx, keepalive](SinkReplyBuilder* builder) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

issues:

  1. Use a move-only functor so we can use unique_ptr
  2. Automatically implement a keepalive mechanism for the transaction

return (prev_state & ASYNC_REPLY_DONE) != 0;
}
bool IsReady() const; // If deferred is ready
bool OnCompletion(util::fb2::detail::Waiter* waiter);
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does it return?


// Update head state and subscribe to updates if its not ready
if (parsed_head_ && parsed_head_ != parsed_to_execute_ && !head_ready)
head_ready |= parsed_head_->OnCompletion(&head_waiter);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please write explanation here what exactly we do here. It's gentle, so I prefer we have comments than going through low-level code to understand

task.blocker->OnCompletion(waiter);
return false;
}};
return visit(ov, reply_);
Copy link
Collaborator

Choose a reason for hiding this comment

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

how the access to reply_ is synchronized between threads?
before, with state_ one a single thread could access reply_payload.
now I see OnCompletion is called from the io thread but you also write into reply_ from the shard thread

Copy link
Contributor Author

@dranikpg dranikpg Jan 9, 2026

Choose a reason for hiding this comment

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

I never write into reply from the shard thread, its either assigned to the return value of the handler or set to the callback that will reply

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I will need to see SET command I guess - how it propagates the Stored reply from the shard thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it won't propagate from the shard thread, it will work via a replier

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
Copilot AI review requested due to automatic review settings January 10, 2026 18:18

This comment was marked as spam.

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