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

Use Foreign Keys in Db*State #8930

Closed
14 of 15 tasks
lenaschoenburg opened this issue Mar 18, 2022 · 9 comments
Closed
14 of 15 tasks

Use Foreign Keys in Db*State #8930

lenaschoenburg opened this issue Mar 18, 2022 · 9 comments
Assignees
@lenaschoenburg
Labels
kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc.
Milestone
Verifying Process...

Comments

@lenaschoenburg
Copy link
Member

lenaschoenburg commented Mar 18, 2022 •
edited
Loading

Let's go over each Db*State and see where we can use foreign keys.

relates to #8884
@lenaschoenburg lenaschoenburg added the kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. label Mar 18, 2022
@lenaschoenburg lenaschoenburg self-assigned this Mar 18, 2022
@lenaschoenburg lenaschoenburg mentioned this issue Mar 18, 2022
Merged
ghost pushed a commit that referenced this issue Mar 18, 2022
@lenaschoenburg
merge: #8932
6167fbd 
8932: refactor: use foreign keys for incidents r=oleschoenburg a=oleschoenburg

## Description

First `DbForeignKey` usage in `DbIncidentState` for referring to element instance and jobs.

I had to adjust the tests to create necessary jobs and element instances.

part of #8930

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
@pihme pihme added this to In progress in Zeebe Mar 18, 2022
@lenaschoenburg lenaschoenburg mentioned this issue Mar 18, 2022
Merged
ghost pushed a commit that referenced this issue Mar 18, 2022
@lenaschoenburg
merge: #8933
68df104 
8933: refactor: use foreign keys for jobs r=oleschoenburg a=oleschoenburg

## Description

Since `DbJobState` contains multiple column families which re-use the `jobKey`, I've added a new `fkJob` which wraps the `jobKey`.

relates to #8930 

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
@lenaschoenburg
Copy link
Member Author

lenaschoenburg commented Mar 18, 2022 •
edited
Loading

Changing DbTimerInstanceState to use a foreign key to element instances will be tricky. If I understand correctly, the timer is created before the element is created. Actually, the elementInstanceKey is first set to -1 and then later updated to the real elementInstanceKey. I'm not sure if and how we could change this.

This was referenced Mar 21, 2022
Merged
Closed
@lenaschoenburg
Copy link
Member Author

lenaschoenburg commented Mar 21, 2022 •
edited
Loading

There's not much to do for DbProcessState except that ZbColumnFamilies.PROCESS_CACHE_DIGEST_BY_ID should use a processId that exists in ZbColumnFamilies.PROCESS_CACHE_BY_ID_AND_VERSION. But ZbColumnFamilies.PROCESS_CACHE_BY_ID_AND_VERSION uses a composite key (process id and version), which is not expressible with DbForeignKey.

I've opened an issue to support this in the future: #8944

@lenaschoenburg
Copy link
Member Author

For DbElementInstance there are two problems:

In ZbColumnFamilies.ELEMENT_INSTANCE_PARENT_CHILD, parentKey should point to an existing element instance so we could use a foreign key there except if there is no parent which is represented with a -1.

ZbColumnFamilies.AWAIT_WORKLOW_RESULT should also use a foreign key to point to an existing element instance but it can't because (apparently) the await metadata is set before the coresponding process instance is created.

ghost pushed a commit that referenced this issue Mar 21, 2022
@korthout @lenaschoenburg
merge: #8919 #8939
e551ed2 
8919: Support decision table inputs and outputs without names/labels r=korthout a=korthout

## Description

<!-- Please explain the changes you made here. -->
A decision tables' inputs and outputs must have an id and can have a name. The name (or label) is not mandatory.

Even though the internal decision engine was already able to deal with this, the workflow engine wasn't. It would try to write the EvaluationEvent record with an EvaluatedInputRecord (or EvaluatedOutputRecord) and then fail writing it because of a NullPointerException.

This makes sure these records can be written when the name of the input or output is undefined.

## Related issues

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

closes #8909



8939: refactor: use foreign keys for decisions r=oleschoenburg a=oleschoenburg

## Description

Uses `DbForeignKey` internally to maintain referential integrity within `DbDecisionState`

relates to #8930 

Co-authored-by: Nico Korthout <nico.korthout@camunda.com>
Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
@lenaschoenburg lenaschoenburg mentioned this issue Mar 21, 2022
Merged
ghost pushed a commit that referenced this issue Mar 21, 2022
@lenaschoenburg
merge: #8939
11cf919 
8939: refactor: use foreign keys for decisions r=oleschoenburg a=oleschoenburg

## Description

Uses `DbForeignKey` internally to maintain referential integrity within `DbDecisionState`

relates to #8930 

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
ghost pushed a commit that referenced this issue Mar 22, 2022
@lenaschoenburg
merge: #8950
5065b08 
8950: refactor: use foreign key for messages r=oleschoenburg a=oleschoenburg

## Description

Uses `DbForeignKey` for referring to existing messages by key.

There are more opportunities to use foreign keys in `DbMessageState`, for example `nameAndCorrelationKey` is reused, but that would depend on #8944 

relates to #8930 

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
@lenaschoenburg
Copy link
Member Author

I think for DbMessageSubscriptionState's elementInstanceKey we can't use a foreign key that points to ZbColumnFamilies.ELEMENT_INSTANCE_KEY. If I try that, there are a lot of failing tests. From the look of it, the message subscription is created before the element instance is inserted. Does that make sense to you @pihme?

@pihme
Copy link
Contributor

pihme commented Mar 22, 2022

I think for DbMessageSubscriptionState's elementInstanceKey we can't use a foreign key that points to ZbColumnFamilies.ELEMENT_INSTANCE_KEY. If I try that, there are a lot of failing tests. From the look of it, the message subscription is created before the element instance is inserted. Does that make sense to you @pihme?

Can you verify that these failing tests are based on a multi partition setup? If that is the case, it does make sense, because then the element instance of that key lives in another partition. If the tests are single partition, we should look deeper what is going on.

@lenaschoenburg
Copy link
Member Author

Can you verify that these failing tests are based on a multi partition setup?

That seems to be the case 👍 So my reasoning was wrong, but the result is the same: we can't use foreign keys for DbMessageSubscriptionState's elementInstanceKey.

@KerstinHebel KerstinHebel removed this from In progress in Zeebe Mar 23, 2022
This was referenced Mar 23, 2022
Merged
Merged
zeebe-bors-camunda bot added a commit that referenced this issue Mar 23, 2022
@zeebe-bors-camunda @lenaschoenburg
merge: #8969
2ac8b0d 
8969: refactor: use foreign keys for element instance parent/child relations r=oleschoenburg a=oleschoenburg

## Description

Uses `DbForeignKey` in `DbElementInstanceState` for the parent and child keys that should point to a valid element instance.

relates to #8930 

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
This was referenced Mar 23, 2022
Merged
Merged
@lenaschoenburg
Copy link
Member Author

@pihme Almost done! I'm lacking a bit of context to assess whether we could or should foreign keys in DbMigrationState. Is that something you could look into for me? 🙇

@pihme
Copy link
Contributor

pihme commented Mar 23, 2022

@pihme Almost done! I'm lacking a bit of context to assess whether we could or should foreign keys in DbMigrationState. Is that something you could look into for me? bow

Let's discuss this together. I had a look. I don't think we need to do anything. I am also reluctant to make any changes here, because this happens before normal stream processing. So exceptions here, and their escalation would be fatal.

At the same time, we need to make sure not to add inconsistencies during the migration.

In any case, I want to discuss it with, state my reasoning and then you can play devil's advocate.

zeebe-bors-camunda bot added a commit that referenced this issue Mar 23, 2022
@zeebe-bors-camunda @lenaschoenburg
merge: #8971
8534fd2 
8971: refactor: use foreign keys for timers r=oleschoenburg a=oleschoenburg

## Description

Timer instances refer to element instances. An exception is made in case the element instance key is `-1` because timers sometimes created before the element.

relates to #8930 

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
zeebe-bors-camunda bot added a commit that referenced this issue Mar 23, 2022
@zeebe-bors-camunda @lenaschoenburg
merge: #8970
dd7d3b7 
8970: refactor: use foreign keys for process state r=oleschoenburg a=oleschoenburg

## Description

Uses `DbForeignKey` for the process state. First usage of prefix matching, which was introduced here: #8960 

relates to #8930 

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
zeebe-bors-camunda bot added a commit that referenced this issue Mar 24, 2022
@zeebe-bors-camunda @lenaschoenburg
merge: #8970
e7eb0d9 
8970: refactor: use foreign keys for process state r=oleschoenburg a=oleschoenburg

## Description

Uses `DbForeignKey` for the process state. First usage of prefix matching, which was introduced here: #8960 

relates to #8930 

Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
@lenaschoenburg
Copy link
Member Author

ZeebeDbInconsistentExceptions during migration behave exactly as exceptions during processing: StreamProcessor#onFailure is called, causing a transition to INACTIVE. Since DbMigrationState reuses other state classes which in turn already use foreign keys, there is nothing to do for DbMigrationState 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lenaschoenburg lenaschoenburg

Labels
kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc.
Projects
None yet
Milestone
Verifying Processing Correctness
Development

No branches or pull requests
3 participants
@npepinpe @lenaschoenburg @pihme