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

test(broker): wait until records are exported before taking snapshot #13039

Merged
1 commit merged into from
Jun 12, 2023

Conversation

deepthidevaki
Copy link
Contributor

@deepthidevaki deepthidevaki commented Jun 9, 2023

Description

The index of snapshot depends both on processed and exported positions. When the exporter is slow, the snapshot is taken at a different index, which resulted in flaky tests. To prevent flakiness, wait until all records are exported before taking the snapshot.

Alternatively, we could disable exporter for this test. But this was not straightforward, as the recording exporter is part of the the rule.

Related issues

closes #12339

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.)
  • 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.

Other teams:
If the change impacts another team an issue has been created for this team, explaining what they need to do to support this change.

Please refer to our review guidelines.

The index of snapshot depends both on processed and exported positions.
When the exporter is slow, the snapshot is taken at a different index,
which resulted in flaky tests. To prevent flakiness, wait until all
records are exported before taking the snapshot.
Copy link
Member

@ChrisKujawa ChrisKujawa left a comment

Choose a reason for hiding this comment

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

Thanks @deepthidevaki 🚀

@deepthidevaki
Copy link
Contributor Author

bors merge

ghost pushed a commit that referenced this pull request Jun 12, 2023
13039: test(broker): wait until records are exported before taking snapshot r=deepthidevaki a=deepthidevaki

## Description

The index of snapshot depends both on processed and exported positions. When the exporter is slow, the snapshot is taken at a different index, which resulted in flaky tests. To prevent flakiness, wait until all records are exported before taking the snapshot.

Alternatively, we could disable exporter for this test. But this was not straightforward, as the recording exporter is part of the the rule.

## Related issues

closes #12339 



Co-authored-by: Deepthi Devaki Akkoorath <[email protected]>
@ghost
Copy link

ghost commented Jun 12, 2023

Build failed:

@deepthidevaki
Copy link
Contributor Author

bors retry

ghost pushed a commit that referenced this pull request Jun 12, 2023
13039: test(broker): wait until records are exported before taking snapshot r=deepthidevaki a=deepthidevaki

## Description

The index of snapshot depends both on processed and exported positions. When the exporter is slow, the snapshot is taken at a different index, which resulted in flaky tests. To prevent flakiness, wait until all records are exported before taking the snapshot.

Alternatively, we could disable exporter for this test. But this was not straightforward, as the recording exporter is part of the the rule.

## Related issues

closes #12339 



Co-authored-by: Deepthi Devaki Akkoorath <[email protected]>
@ghost
Copy link

ghost commented Jun 12, 2023

Build failed:

@deepthidevaki
Copy link
Contributor Author

bors merge

@ghost
Copy link

ghost commented Jun 12, 2023

Build succeeded:

@ghost ghost merged commit 51c9302 into main Jun 12, 2023
@ghost ghost deleted the dd-12339-snapshot-test branch June 12, 2023 10:19
@backport-action
Copy link
Collaborator

Backport failed for stable/8.0, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin stable/8.0
git worktree add -d .worktree/backport-13039-to-stable/8.0 origin/stable/8.0
cd .worktree/backport-13039-to-stable/8.0
git checkout -b backport-13039-to-stable/8.0
ancref=$(git merge-base d166007d8fee3fa6f112367ea595d35199807f4f 1edc2a246ec51894e900505336d2ab6c7ff7c018)
git cherry-pick -x $ancref..1edc2a246ec51894e900505336d2ab6c7ff7c018

@backport-action
Copy link
Collaborator

@backport-action
Copy link
Collaborator

ghost pushed a commit that referenced this pull request Jun 12, 2023
13059: [Backport stable/8.1] test(broker): wait until records are exported before taking snapshot r=deepthidevaki a=backport-action

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

relates to #12339

Co-authored-by: Deepthi Devaki Akkoorath <[email protected]>
ghost pushed a commit that referenced this pull request Jun 12, 2023
13060: [Backport stable/8.2] test(broker): wait until records are exported before taking snapshot r=deepthidevaki a=backport-action

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

relates to #12339

Co-authored-by: Deepthi Devaki Akkoorath <[email protected]>
@lenaschoenburg lenaschoenburg added the version:8.2.7 Marks an issue as being completely or in parts released in 8.2.7 label Jun 13, 2023
ghost pushed a commit that referenced this pull request Jun 14, 2023
13087: [Backport stable 8.0] test(broker): wait until records are exported before taking snapshot r=deepthidevaki a=deepthidevaki

Backport #13039 

closes #12339

Co-authored-by: Deepthi Devaki Akkoorath <[email protected]>
@ChrisKujawa ChrisKujawa added release/8.0.18 version:8.1.14 Marks an issue as being completely or in parts released in 8.1.14 labels Jul 6, 2023
github-merge-queue bot pushed a commit that referenced this pull request Aug 9, 2024
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.14 Marks an issue as being completely or in parts released in 8.1.14 version:8.2.7 Marks an issue as being completely or in parts released in 8.2.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BrokerSnapshotTest.shouldTakeSnapshotAtCorrectIndex flaky
4 participants