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

feat: run timer due date checker concurrently to processing #12403

Merged
merged 3 commits into from
Apr 17, 2023

Conversation

lenaschoenburg
Copy link
Member

@lenaschoenburg lenaschoenburg commented Apr 13, 2023

This follows a pattern we established for message TTL checking and introduces a new experimental feature flag
zeebe.broker.experimental.features.enableTimerDueDateCheckerAsync. When enabled, timer due dates are checked outside of the processing actor.
This ensures that a large number of expired timers does not block other processing and thus cause latency spikes.

Again similar to message TTL checking, there is another experimental configuration that enables yielding of the timer due date checker after a fixed amount of time has passed. This is conceptually similar to ttlCheckerBatchLimit in that it ensures that a single run of a scheduled task does not block for too long and produces a limited amount of new records.
Yielding continues to function the same, regardless of whether async checking is enabled or not.

closes #11594

@lenaschoenburg lenaschoenburg force-pushed the os-11594-async-due-timer-triggering branch from efdf474 to 771a079 Compare April 13, 2023 13:03
@lenaschoenburg lenaschoenburg added backport stable/8.1 backport stable/8.2 Backport a pull request to 8.2.x labels Apr 13, 2023
This state is used by the timer due date checker scheduled task and
should not be shared with regular processing.
@github-actions
Copy link
Contributor

github-actions bot commented Apr 13, 2023

Test Results

1 054 files  ±    0  1 054 suites  ±0   1h 58m 9s ⏱️ - 2m 20s
8 444 tests  - 106  8 418 ✔️  - 106  26 💤 ±0  0 ±0 
8 652 runs   - 106  8 626 ✔️  - 106  26 💤 ±0  0 ±0 

Results for commit 1093893. ± Comparison against base commit 1d32130.

This pull request removes 566 and adds 460 tests. Note that renamed tests count towards both.
DmnEvaluationTest If successfully evaluated, the output ‑ Should return a message pack output[6] value={z=[1, 2, 3], y=true, x=1}
io.camunda.zeebe.engine.processing.bpmn.activity.OutputMappingTest ‑ shouldApplyOutputMapping[0: io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@cae60f1]
io.camunda.zeebe.engine.processing.bpmn.activity.OutputMappingTest ‑ shouldApplyOutputMapping[1: io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@4e8fab14]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=BUSINESS_RULE_TASK, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@50cf26b7, variables={}]]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=CALL_ACTIVITY, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@419df81e, variables={}]]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=END_EVENT, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@445760e0, variables={}]]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=EVENT_BASED_GATEWAY, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@507c33cc, variables={correlationKey=value}]]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=EVENT_SUB_PROCESS, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@b6db92b, variables={}]]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=EXCLUSIVE_GATEWAY, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@24e5a4e0, variables={}]]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=INTERMEDIATE_CATCH_EVENT, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@15500a8, variables={correlationKey=value}]]
…
DmnEvaluationTest If successfully evaluated, the output ‑ Should return a message pack output[6] value={x=1, y=true, z=[1, 2, 3]}
io.camunda.zeebe.broker.system.configuration.FeatureFlagsCfgTest ‑ shouldDisableDueDateCheckerAsyncByDefault
io.camunda.zeebe.broker.system.configuration.FeatureFlagsCfgTest ‑ shouldSetEnableTimerDueDateCheckerAsyncFromConfig
io.camunda.zeebe.broker.system.configuration.FeatureFlagsCfgTest ‑ shouldSetEnableTimerDueDateCheckerAsyncFromEnv
io.camunda.zeebe.engine.processing.bpmn.activity.OutputMappingTest ‑ shouldApplyOutputMapping[0: io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@12eb281f]
io.camunda.zeebe.engine.processing.bpmn.activity.OutputMappingTest ‑ shouldApplyOutputMapping[1: io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@6621ab0c]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=BUSINESS_RULE_TASK, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@ebcf6c0, variables={}]]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=CALL_ACTIVITY, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@6e774194, variables={}]]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=END_EVENT, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@3e49adfa, variables={}]]
io.camunda.zeebe.engine.processing.processinstance.CreateProcessInstanceSupportedElementTest ‑ testProcessInstanceCanStartAtElementType[Scenario[type=EVENT_BASED_GATEWAY, modelInstance=io.camunda.zeebe.model.bpmn.impl.BpmnModelInstanceImpl@67954d71, variables={correlationKey=value}]]
…

♻️ This comment has been updated with latest results.

This copies the scheme already used for async message TTL checking
and adds a new experimental feature flag.
It is disabled by default for safety but enabled in tests so that we can
find potential issues faster.
This uses the new config `enableTimerDueDateCheckerAsync` to switch
between running the timer due date checker scheduled task on the
processing actor or another actor.

The reasoning is the same as for running message TTL checking outside of
the processing actor: Some use cases have a lot of messages/timers that
need to be checked and that potentially expire at the same time. This
should not block regular processing so these tasks must not block the
processing actor.

Timer due date checking already supported an experimental feature
to interrupt a single check run after a certain amount of time has
passed. Conceptually this is similar to the batching that we implemented
for message TTL checking. This feature continues to function, regardless
of whether async checking is enabled or not.
@lenaschoenburg lenaschoenburg force-pushed the os-11594-async-due-timer-triggering branch from 771a079 to 1093893 Compare April 13, 2023 13:37
@lenaschoenburg lenaschoenburg marked this pull request as ready for review April 13, 2023 13:57
Copy link
Member

@korthout korthout left a comment

Choose a reason for hiding this comment

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

👏 Clean changes and well-argued commit messages 😍

👍 Test coverage is met because the feature flag is enabled for tests

LGTM 👍

@lenaschoenburg
Copy link
Member Author

bors r+

@zeebe-bors-camunda
Copy link
Contributor

Build succeeded:

@zeebe-bors-camunda zeebe-bors-camunda bot merged commit f402edc into main Apr 17, 2023
27 checks passed
@zeebe-bors-camunda zeebe-bors-camunda bot deleted the os-11594-async-due-timer-triggering branch April 17, 2023 16:17
@backport-action
Copy link
Collaborator

Backport failed for stable/8.1, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin stable/8.1
git worktree add -d .worktree/backport-12403-to-stable/8.1 origin/stable/8.1
cd .worktree/backport-12403-to-stable/8.1
git checkout -b backport-12403-to-stable/8.1
ancref=$(git merge-base 1d321300618431b49259f490ced2202b5f78a321 1093893e7b0aa27849677218d1ac32738e77fa3b)
git cherry-pick -x $ancref..1093893e7b0aa27849677218d1ac32738e77fa3b

@backport-action
Copy link
Collaborator

Successfully created backport PR for stable/8.2:

zeebe-bors-camunda bot added a commit that referenced this pull request Apr 17, 2023
12454: [Backport stable/8.2] feat: run timer due date checker concurrently to processing r=oleschoenburg a=backport-action

# Description
Backport of #12403 to `stable/8.2`.

relates to #11594

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
zeebe-bors-camunda bot added a commit that referenced this pull request Apr 18, 2023
12467: [Backport stable/8.1] feat: run timer due date checker concurrently to processing r=oleschoenburg a=oleschoenburg

Manual backport of #12403.
No tricky merge conflicts, just minor differences in naming and the list of available feature flags.

12468: [Backport 8.1]: skip unnecessary blacklist check r=megglos a=Zelldon

## Description

Backport  #12306
<!-- Please explain the changes you made here. -->

## Related issues

<!-- Which issues are closed by this PR or are related -->

closes to #12041



Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
Co-authored-by: Meggle (Sebastian Bathke) <sebastian.bathke@camunda.com>
zeebe-bors-camunda bot added a commit that referenced this pull request Apr 18, 2023
12467: [Backport stable/8.1] feat: run timer due date checker concurrently to processing r=oleschoenburg a=oleschoenburg

Manual backport of #12403.
No tricky merge conflicts, just minor differences in naming and the list of available feature flags.

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
zeebe-bors-camunda bot added a commit that referenced this pull request Apr 19, 2023
12467: [Backport stable/8.1] feat: run timer due date checker concurrently to processing r=oleschoenburg a=oleschoenburg

Manual backport of #12403.
No tricky merge conflicts, just minor differences in naming and the list of available feature flags.

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
@lenaschoenburg lenaschoenburg added the version:8.2.3 Marks an issue as being completely or in parts released in 8.2.3 label Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport stable/8.2 Backport a pull request to 8.2.x version:8.2.3 Marks an issue as being completely or in parts released in 8.2.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Triggering due timer events causes periodic latency spikes
3 participants