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(broker): report health on change only #13655

Merged
1 commit merged into from
Jul 27, 2023
Merged

fix(broker): report health on change only #13655

1 commit merged into from
Jul 27, 2023

Conversation

megglos
Copy link
Contributor

@megglos megglos commented Jul 27, 2023

Description

Previously every invocation of updateHealthStatus caused listeners to be called even if nothing changed.

Related issues

closes #13650

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

Previously every invocation of updateHealthStatus caused listeners to be called even if nothing changed.
zeebePartitionHealth.addFailureListener(failureListener);

// when
partition.onNewRole(Role.LEADER, 1);

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [ZeebePartition.onNewRole](1) should be avoided because it has been deprecated.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oleschoenburg is there any alternative to this?

Copy link
Member

Choose a reason for hiding this comment

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

No 😭 This is just the way it is currently and why the partition transition process needs some work.

@megglos megglos requested a review from lenaschoenburg July 27, 2023 06:29
Copy link
Member

@lenaschoenburg lenaschoenburg left a comment

Choose a reason for hiding this comment

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

Thanks @megglos 🚀

@megglos
Copy link
Contributor Author

megglos commented Jul 27, 2023

bors merge

ghost pushed a commit that referenced this pull request Jul 27, 2023
13655: fix(broker): report health on change only r=megglos a=megglos

## Description

Previously every invocation of updateHealthStatus caused listeners to be called even if nothing changed.

## Related issues

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

closes #13650 



Co-authored-by: Meggle (Sebastian Bathke) <[email protected]>
@ghost
Copy link

ghost commented Jul 27, 2023

Build failed:

@megglos
Copy link
Contributor Author

megglos commented Jul 27, 2023

bors retry

@ghost
Copy link

ghost commented Jul 27, 2023

Build succeeded:

@ghost ghost merged commit ebe26f8 into main Jul 27, 2023
@ghost ghost deleted the meg-13650-log-health-once branch July 27, 2023 07:10
@backport-action
Copy link
Collaborator

Successfully created backport PR for stable/8.0:

@backport-action
Copy link
Collaborator

Successfully created backport PR for stable/8.1:

@backport-action
Copy link
Collaborator

Successfully created backport PR for stable/8.2:

ghost pushed a commit that referenced this pull request Jul 27, 2023
13656: [Backport stable/8.0] fix(broker): report health on change only r=megglos a=backport-action

# Description
Backport of #13655 to `stable/8.0`.

relates to #13650

Co-authored-by: Meggle (Sebastian Bathke) <[email protected]>
ghost pushed a commit that referenced this pull request Jul 27, 2023
13658: [Backport stable/8.2] fix(broker): report health on change only r=megglos a=backport-action

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

relates to #13650

Co-authored-by: Meggle (Sebastian Bathke) <[email protected]>
ghost pushed a commit that referenced this pull request Jul 27, 2023
13657: [Backport stable/8.1] fix(broker): report health on change only r=megglos a=backport-action

# Description
Backport of #13655 to `stable/8.1`.

relates to #13650

Co-authored-by: Meggle (Sebastian Bathke) <[email protected]>
ghost pushed a commit that referenced this pull request Jul 27, 2023
13657: [Backport stable/8.1] fix(broker): report health on change only r=megglos a=backport-action

# Description
Backport of #13655 to `stable/8.1`.

relates to #13650

Co-authored-by: Meggle (Sebastian Bathke) <[email protected]>
@megglos megglos added version:8.2.10 Marks an issue as being completely or in parts released in 8.2.10 release/8.0.19 labels Jul 27, 2023
@abbasadel abbasadel added the version:8.1.15 Marks an issue as being completely or in parts released in 8.1.15 label Aug 2, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version:8.1.15 Marks an issue as being completely or in parts released in 8.1.15 version:8.2.10 Marks an issue as being completely or in parts released in 8.2.10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ZeebePartitionHealth repeatedly reports change of health status
4 participants