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

Fix a bug with writing history before the history event is created. #5476

Merged
merged 5 commits into from
Nov 30, 2023

Conversation

dmitrii-beliakov
Copy link
Collaborator

@dmitrii-beliakov dmitrii-beliakov commented Nov 15, 2023

This issue addresses the problem that some history actions are created before a history event is created and since there is a FK relationship between actions and events, those actions are not saved and the appropriate error is written in the logs.

The simplest reproduction steps are just trying to execute an operation for non-existing flow, then the response contain an error message sating that an operation cannot be executed because the flow was not found. At the same time there is an error in the logs saying that event_id cannot be null. When we navigate to the history of that flow, we can see that this information is not saved. There could be other use cases, essentially when an error happens before the event is created. Usually, the history event is created after a flow validation action.

After this fix, a history event is created before any other actions, so all errors and other actions are saved in the history and there is no error in the logs.

Reproductions steps: for creation, create a flow, then create another flow with the same request, but different flow ID. For other operations execute them with any valid request parameters, but for non-existing flow. Then, navigate to the history of that flow, the history is saved even for non-existing flows.

Affected operations:
Simple flow:

  • Flow create
  • Flow update
  • Flow delete
  • Flow mirror point create
  • Flow mirror point delete
  • Flow path swap
  • Flow reroute ?
  • Flow path swap
  • Flow swap endpoints

Y-flow:

  • Y-flow create
  • Y-flow update
  • Y-flow delete
  • Y-flow path swap
  • Y-flow reroute ?

HA-flow:

  • HA-flow create
  • HA-flow update
  • HA-flow delete
  • HA-flow path swap
  • HA-flow reroute ?
  • HA-flow sync

(*? reroute was prone to the same error, however I wasn't able to find simple reprosteps)

closes #4738

@@ -55,7 +55,8 @@ public enum Event {
PARTIAL_UPDATE("HA-Flow partial update"),
REROUTE("HA-Flow reroute"),
DELETE("HA-Flow delete"),
PATH_SWAP("HA-flow path swap");
PATH_SWAP("HA-flow path swap"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's be consistent and use 'HA-Flow' at least within the same enum.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I corrected the description

@pkazlenka
Copy link
Collaborator

Fix was tested manually, but needs to pass regression before the merge.

Copy link
Collaborator

@IvanChupin IvanChupin left a comment

Choose a reason for hiding this comment

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

Great job! 👍

@niksv niksv merged commit 883d70a into develop Nov 30, 2023
@niksv niksv deleted the fix/history_event branch November 30, 2023 09:57
@niksv niksv removed the Next Release label Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creation of flow with endpoint conflicts causes SQLIntegrityConstraintViolationException in history bolt
4 participants