Consider the current stage when applying Action::Complete to the vqueue meta #4071
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The vqueue items table is intended to store the payload of a vqueue item (e.g. the invocation or the state mutation). The item is stored when it is being enqueued into the vqueue and removed once the vqueue item ends. The payload can be accessed by the tuple (qid, created_at, kind, entry_id). The creation timestamp is part of the key in order to establish the insertion order into the given vqueue.
This commit integrates vqueues with external state mutations. The way it works is by storing the external state mutation in the VQueueItems table until it gets run by the scheduler. When running the state mutation, we retrieve it from the VQueueItems table and apply the state mutation changes. The external state mutation EntryId is currently derived from the internal inbox_seq_number to generate unique EntryId's. In the future we might give external state mutations an explicit id which could be used to derive the EntryId. Currently, the DRR scheduler reserves a concurrency token for running state mutations. We might want to relax this condition if we see that applying state mutations does not need to be protected by the global concurrency limit.
Before, the DRR scheduler checked for eligibility if the current head item was removed. This was not enough since a vqueue might have been not eligible because it was running out of concurrency tokens. A removed item could have released the concurrency token and thereby it could make the vqueue eligible again if the vqueue contains more items. Hence, we should check on every removed item whether this event had an impact on the vqueue's eligibility.
Since we calculate the partition key for VOs and workflows only from the key, we need to include the VO/workflow name when creating the VQueueId (for generating an instance id which does not conflict with other VOs/ workflows). This is a bit unfortunate :-(
…ue meta When applying Action::Complete to the vqueue meta we need to consider what's the current stage to properly update the statistics and the metadata. Hence, we are now passing the stage instead of whether it was in the inbox or not because we need to handle `Stage::Run` and `Stage::Park` differently.
tillrohrmann
force-pushed
the
vqueues/fix-vqueue-id-generation
branch
from
ece31e4 to
5d91253
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Include VO/workflow name in vqueue id generation
Since we calculate the partition key for VOs and workflows only from the
key, we need to include the VO/workflow name when creating the VQueueId
(for generating an instance id which does not conflict with other VOs/
workflows). This is a bit unfortunate :-(
Consider the current stage when applying Action::Complete to the vqueue meta
When applying Action::Complete to the vqueue meta we need to consider what's the current stage
to properly update the statistics and the metadata. Hence, we are now passing the stage instead of whether it was in the inbox or not because we need to handle
Stage::RunandStage::Parkdifferently.This PR is based on #4070.