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.

Sign up for GitHub

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

Jump to bottom

Separate StreamProcessorContext and TypedRecordProcessorContext #9826

Merged
merged 4 commits into from
Jul 18, 2022
Merged

Separate StreamProcessorContext and TypedRecordProcessorContext #9826

merged 4 commits into from
Jul 18, 2022

Conversation

pihme
Copy link
Contributor

@pihme pihme commented Jul 18, 2022

Description

  • This separates the context objects needed by the stream processor vs. the engine.
  • They still are used to exchange information back and forth (i.e. setWriters()), which is a bit of a smell. Some of the information flows will be removed by further refactoring.

Related issues

closes #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.

@pihme
refactor(engine): implement TypedRecordProcessorContext
65844b2
@pihme
refactor(engine): define RecordProcessorContext
f525aa0
@pihme
refactor(engine): implement RecordProcessorContext
7636e38
@pihme
refactor(engine): StreamProcessorContext no longer implements TypedRe…
b5edab6 
…cordProcessorContext

This is to separate the two contexts. One is used for the stream processor, the other in the engine.

This is an intermediate step where the engine upon initializing
returns objects that are still needed in the stream processor
context. SOme will disappear once the engine abstraction
refactoring is complete. Others may require additional refactoring
later on.
@pihme pihme requested a review from npepinpe July 18, 2022 11:27
pihme
pihme commented Jul 18, 2022
View reviewed changes
engine/src/main/java/io/camunda/zeebe/streamprocessor/StreamProcessorBuilder.java
@@ -128,8 +128,6 @@ private void validate() {
Objects.requireNonNull(typedRecordProcessorFactory, "No stream processor factory provided.");
Objects.requireNonNull(actorSchedulingService, "No task scheduler provided.");
Objects.requireNonNull(streamProcessorContext.getLogStream(), "No log stream provided.");
Objects.requireNonNull(
Copy link
Contributor Author

@pihme pihme Jul 18, 2022 •
edited

Choose a reason for hiding this comment

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

This line was removed due to lifecycle changes (i.e. the field will now be set at a later time).

github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 18, 2022
View reviewed changes
engine/src/main/java/io/camunda/zeebe/engine/Engine.java
recordProcessorContext.getScheduleService(),
recordProcessorContext.getZeebeDb(),
recordProcessorContext.getTransactionContext(),
recordProcessorContext.getStreamWriterProxy(),

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [RecordProcessorContext.getStreamWriterProxy](1) should be avoided because it has been deprecated.
engine/src/main/java/io/camunda/zeebe/engine/Engine.java
recordProcessorContext.getTransactionContext(),
recordProcessorContext.getStreamWriterProxy(),
recordProcessorContext.getEventApplierFactory(),
recordProcessorContext.getTypedResponseWriter());

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [RecordProcessorContext.getTypedResponseWriter](1) should be avoided because it has been deprecated.
engine/src/main/java/io/camunda/zeebe/engine/Engine.java
recordProcessorContext.setStreamProcessorListener(
typedProcessorContext.getStreamProcessorListener());

recordProcessorContext.setLifecycleListeners(typedRecordProcessors.getLifecycleListeners());

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [RecordProcessorContext.setLifecycleListeners](1) should be avoided because it has been deprecated.
engine/src/main/java/io/camunda/zeebe/engine/Engine.java
recordProcessorContext.setLifecycleListeners(typedRecordProcessors.getLifecycleListeners());
final RecordProcessorMap recordProcessorMap = typedRecordProcessors.getRecordProcessorMap();

recordProcessorContext.setRecordProcessorMap(recordProcessorMap);

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [RecordProcessorContext.setRecordProcessorMap](1) should be avoided because it has been deprecated.
engine/src/main/java/io/camunda/zeebe/engine/Engine.java
final RecordProcessorMap recordProcessorMap = typedRecordProcessors.getRecordProcessorMap();

recordProcessorContext.setRecordProcessorMap(recordProcessorMap);
recordProcessorContext.setWriters(typedProcessorContext.getWriters());

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [RecordProcessorContext.setWriters](1) should be avoided because it has been deprecated.
engine/src/main/java/io/camunda/zeebe/streamprocessor/StreamProcessor.java
typedRecordProcessorFactory);
engine.init(recordProcessorContext);

lifecycleAwareListeners.addAll(recordProcessorContext.getLifecycleListeners());

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [RecordProcessorContextImpl.getLifecycleListeners](1) should be avoided because it has been deprecated.
engine/src/main/java/io/camunda/zeebe/streamprocessor/StreamProcessor.java
engine.init(recordProcessorContext);

lifecycleAwareListeners.addAll(recordProcessorContext.getLifecycleListeners());
streamProcessorContext.recordProcessorMap(recordProcessorContext.getRecordProcessorMap());

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [RecordProcessorContextImpl.getRecordProcessorMap](1) should be avoided because it has been deprecated.
@github-actions
Copy link
Contributor

Unit Test Results

   792 files  ±    0     792 suites  ±0   1h 36m 34s ⏱️ + 1m 30s
5 998 tests +105  5 989 ✔️ +105  9 💤 ±0  0 ❌ ±0 
6 167 runs  +105  6 158 ✔️ +105  9 💤 ±0  0 ❌ ±0 

Results for commit b5edab6. ± Comparison against base commit 2027c06.

npepinpe
npepinpe approved these changes Jul 18, 2022
View reviewed changes
Copy link
Member

@npepinpe npepinpe left a comment •
edited

Choose a reason for hiding this comment

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

👍

  • 💭 Would be great if we could turn all these context objects into immutable objects, I think. It would make reasoning about the system simpler, and would allow us to turn them in light records.

engine/src/main/java/io/camunda/zeebe/streamprocessor/StreamProcessorContext.java Show resolved Hide resolved
@pihme
Copy link
Contributor Author

pihme commented Jul 18, 2022

Would be great if we could turn all these context objects into immutable objects, I think. It would make reasoning about the system simpler, and would allow us to turn them in light records.

Sound good, but not quite possible as of now

@pihme
Copy link
Contributor Author

pihme commented Jul 18, 2022

bors merge

@zeebe-bors-camunda
Copy link
Contributor

Build succeeded:

@zeebe-bors-camunda zeebe-bors-camunda bot merged commit 3ed7a49 into main Jul 18, 2022
@zeebe-bors-camunda zeebe-bors-camunda bot deleted the 9725-separate-streamprocessorcontest-and-typedrecordprocessorcontext branch July 18, 2022 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@npepinpe npepinpe npepinpe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor StreamProcessor / Engine
2 participants
@pihme @npepinpe