Create engine outside #9989

Zelldon · 2022-08-04T10:44:18Z

Description

Create the engine outside of the StreamProcessor, which should allow to also inject other RecordProcessors.
Furthermore we can fill the Engine with the needed TypedRecordProcessorFactory, which means we no longer need that in our builders and contexts.

I hope this helps you in make further progress @deepthidevaki

Sidenote:

This also simplifies the StreamProcessor/RecordProcessor tests, since we do not necessarily need to set up so much anymore.

Related issues

related to #9725

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

The changes are backwards compatibility with previous versions
If it fixes a bug then PRs are created to backport the fix to the last two minor versions. You can trigger a backport by assigning labels (e.g. backport stable/1.3) to the PR, in case that fails you need to create backports manually.

Testing:

There are unit/integration tests that verify all acceptance criterias of the issue
New tests are written to ensure backwards compatibility with further versions
The behavior is tested manually
The change has been verified by a QA run
The impact of the changes is verified by a benchmark

Documentation:

The documentation is updated (e.g. BPMN reference, configuration, examples, get-started guides, etc.)
New content is added to the release announcement
If the PR changes how BPMN processes are validated (e.g. support new BPMN element) then the Camunda modeling team should be informed to adjust the BPMN linting.

Please refer to our review guidelines.

The streamprocessor builder accepts an RecordProcessor useful to add other RecordProcessors

With creating the engine outside the stream processor we can fill it with all necessary dependency and we no longer need these in the builders and contexts

github-actions · 2022-08-04T11:13:37Z

Test Results

  810 files ±    0   810 suites ±0 1h 37m 4s ⏱️ -49s
6 033 tests - 211 6 022 ✔️ - 213 11 💤 +2 0 ❌ ±0
6 221 runs - 211 6 210 ✔️ - 213 11 💤 +2 0 ❌ ±0

Results for commit fae722d. ± Comparison against base commit d80629f.

♻️ This comment has been updated with latest results.

deepthidevaki

🚀 Exactly what I wanted 😄 Just some minor comment.

deepthidevaki · 2022-08-04T15:03:20Z

engine/src/main/java/io/camunda/zeebe/engine/RecordProcessorContext.java

@@ -21,7 +20,7 @@
 import java.util.List;
 import java.util.function.Function;

-public final class EngineContext {
+public final class RecordProcessorContext {


❌ : Can be moved to package io.camunda.zeebe.engine.api. as this is should not be owned by the "engine".
Or better define an interface in io.camunda.zeebe.engine.api and have the implementation in io.camunda.zeebe.streamprocessor

🔧 You can make it a record.

I don't see that as blocker since we are not done yet but sure can do tomorrow.

Zelldon · 2022-08-04T10:45:59Z

...java/io/camunda/zeebe/broker/system/partitions/impl/steps/StreamProcessorTransitionStep.java

@@ -127,11 +128,11 @@ private static StreamProcessor createStreamProcessor(
        .logStream(context.getLogStream())
        .actorSchedulingService(context.getActorSchedulingService())
        .zeebeDb(context.getZeebeDb())
+        .recordProcessor(new Engine(context.getStreamProcessorFactory()))


🔧 I guess we should rename the factory here

Zelldon · 2022-08-04T15:09:46Z

engine/src/main/java/io/camunda/zeebe/engine/RecordProcessorContext.java

@@ -21,7 +20,7 @@
 import java.util.List;
 import java.util.function.Function;

-public final class EngineContext {
+public final class RecordProcessorContext {


I don't see that as blocker since we are not done yet but sure can do tomorrow.

Zelldon · 2022-08-05T11:56:47Z

@deepthidevaki I moved the implementation and introduced a new interface. Could you review again? Maybe you trust me next time 😁 so I can just go ahead and merge after the changes :)

deepthidevaki · 2022-08-05T12:17:50Z

@deepthidevaki I moved the implementation and introduced a new interface. Could you review again? Maybe you trust me next time grin so I can just go ahead and merge after the changes :)

😕 I didn't know that we now approve and request changes at the same time.

deepthidevaki

Thanks.

engine/src/main/java/io/camunda/zeebe/engine/Engine.java

+    recordProcessorContext.setStreamProcessorListener(
+        typedProcessorContext.getStreamProcessorListener());


Zelldon · 2022-08-05T12:24:17Z

Since this was only something minor I thought so :) https://github.com/camunda/zeebe/blob/main/CONTRIBUTING.md#reviewing-a-pull-request

Zelldon · 2022-08-05T12:27:18Z

bors r+

9989: Create engine outside r=Zelldon a=Zelldon ## Description Create the engine outside of the StreamProcessor, which should allow to also inject other RecordProcessors. Furthermore we can fill the Engine with the needed TypedRecordProcessorFactory, which means we no longer need that in our builders and contexts. I hope this helps you in make further progress `@deepthidevaki` Sidenote: This also simplifies the StreamProcessor/RecordProcessor tests, since we do not necessarily need to set up so much anymore.  ## Related issues  related to #9725 Co-authored-by: Christopher Zell <zelldon91@googlemail.com>

zeebe-bors-camunda · 2022-08-05T13:20:55Z

Build failed:

Test summary

Zelldon · 2022-08-05T13:21:18Z

bors r+

9989: Create engine outside r=Zelldon a=Zelldon ## Description Create the engine outside of the StreamProcessor, which should allow to also inject other RecordProcessors. Furthermore we can fill the Engine with the needed TypedRecordProcessorFactory, which means we no longer need that in our builders and contexts. I hope this helps you in make further progress `@deepthidevaki` Sidenote: This also simplifies the StreamProcessor/RecordProcessor tests, since we do not necessarily need to set up so much anymore.  ## Related issues  related to #9725 10012: refactor: let `dist` build the actor scheduler for gateway r=oleschoenburg a=oleschoenburg ## Description This moves the responsibility of creating an actor scheduler for the gateway to the `dist` module. As an added benefit, this allows us to remove some redundant constructors, simplify BrokerClient to never start the actor scheduler and a deprecated method from `BrokerStartupContext`. ## Related issues  relates to #9996 Co-authored-by: Christopher Zell <zelldon91@googlemail.com> Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>

zeebe-bors-camunda · 2022-08-05T14:02:27Z

This PR was included in a batch that was canceled, it will be automatically retried

zeebe-bors-camunda · 2022-08-05T14:34:52Z

Build succeeded:

Zelldon added 8 commits August 4, 2022 12:13

refactor: rename context

6f03a50

refactor: always accept RecordProcessorContext

58be8da

refactor: create engine outside

4ddfb3a

The streamprocessor builder accepts an RecordProcessor useful to add other RecordProcessors

refactor: create engine with TypedRecordProcessor factory

99a0625

With creating the engine outside the stream processor we can fill it with all necessary dependency and we no longer need these in the builders and contexts

style: follow code style

63a9a2f

style: follow code style

877a5a6

fix: adjust CheckpointProcessor to recent changes

346118d

test: adjust test to recent changes

276831d

Zelldon requested a review from deepthidevaki August 4, 2022 10:44

deepthidevaki requested changes Aug 4, 2022

View reviewed changes

Zelldon commented Aug 4, 2022

View reviewed changes

Zelldon added 2 commits August 5, 2022 13:25

refactor: rename streamProcessorFactory

09fb1eb

refactor: introduce RecordProcessorContext interface

fae722d

Zelldon requested a review from deepthidevaki August 5, 2022 11:56

deepthidevaki approved these changes Aug 5, 2022

View reviewed changes

github-advanced-security bot found potential problems Aug 5, 2022

View reviewed changes

Zelldon mentioned this pull request Aug 5, 2022

Create EventAppliers in engine #9985

Merged

10 tasks

zeebe-bors-camunda bot merged commit e751fd6 into main Aug 5, 2022

zeebe-bors-camunda bot deleted the zell-create-engine-outside branch August 5, 2022 14:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create engine outside #9989

Create engine outside #9989

Zelldon commented Aug 4, 2022 •

edited

github-actions bot commented Aug 4, 2022 •

edited

deepthidevaki left a comment

deepthidevaki Aug 4, 2022

Zelldon Aug 4, 2022

Zelldon Aug 4, 2022

Zelldon Aug 4, 2022

Zelldon commented Aug 5, 2022

deepthidevaki commented Aug 5, 2022

deepthidevaki left a comment

Zelldon commented Aug 5, 2022

Zelldon commented Aug 5, 2022

zeebe-bors-camunda bot commented Aug 5, 2022

Zelldon commented Aug 5, 2022

zeebe-bors-camunda bot commented Aug 5, 2022

zeebe-bors-camunda bot commented Aug 5, 2022

		recordProcessorContext.setStreamProcessorListener(
		typedProcessorContext.getStreamProcessorListener());

Create engine outside #9989

Create engine outside #9989

Conversation

Zelldon commented Aug 4, 2022 • edited

Description

Related issues

Definition of Done

github-actions bot commented Aug 4, 2022 • edited

Test Results

deepthidevaki left a comment

Choose a reason for hiding this comment

deepthidevaki Aug 4, 2022

Choose a reason for hiding this comment

Zelldon Aug 4, 2022

Choose a reason for hiding this comment

Zelldon Aug 4, 2022

Choose a reason for hiding this comment

Zelldon Aug 4, 2022

Choose a reason for hiding this comment

Zelldon commented Aug 5, 2022

deepthidevaki commented Aug 5, 2022

deepthidevaki left a comment

Choose a reason for hiding this comment

Zelldon commented Aug 5, 2022

Zelldon commented Aug 5, 2022

zeebe-bors-camunda bot commented Aug 5, 2022

Zelldon commented Aug 5, 2022

zeebe-bors-camunda bot commented Aug 5, 2022

zeebe-bors-camunda bot commented Aug 5, 2022

Zelldon commented Aug 4, 2022 •

edited

github-actions bot commented Aug 4, 2022 •

edited