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

Increase timeout and add logging #10868

Merged
merged 2 commits into from Oct 31, 2022
Merged

Increase timeout and add logging #10868

merged 2 commits into from Oct 31, 2022

Conversation

npepinpe
Copy link
Member

Description

Increases the timeout of the assertion from 15 to 30 seconds, and adds more diagnostic logging for easier debugging next time this occurs (if it does 🤞).

See the last comment for more, specifically the last paragraph.

Related issues

related to #10234

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.

Please refer to our review guidelines.

Increases the timeout we wait for the topology to be complete from 15
seconds to 30 seconds. The idea is that 15 may be right on the cusp of
the minimum limit we've observed, due to possibly requiring up to 5
seconds for a node to be elected, and hypothetical long GC pauses. This
doesn't not unflake the test, but hopefully the next time it fails we
have a better idea of what is happening.

At the same time, the test is updated to junit 5, and the gateway (from
which we get the topology) is also configured for faster gossip, like
the broker.
@github-actions
Copy link
Contributor

Test Results

   950 files  ±  0     950 suites  ±0   1h 47m 18s ⏱️ + 2m 52s
7 769 tests +38  7 762 ✔️ +38  7 💤 ±0  0 ±0 
7 967 runs  +38  7 958 ✔️ +38  9 💤 ±0  0 ±0 

Results for commit 9d30d71. ± Comparison against base commit b8e2293.

Copy link
Member

@oleschoenburg oleschoenburg left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@npepinpe
Copy link
Member Author

bors merge

@zeebe-bors-camunda
Copy link
Contributor

Build succeeded:

@zeebe-bors-camunda zeebe-bors-camunda bot merged commit 7340a58 into main Oct 31, 2022
@zeebe-bors-camunda zeebe-bors-camunda bot deleted the 10234-flaky branch October 31, 2022 17:09
@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-10868-to-stable/8.0 origin/stable/8.0
cd .worktree/backport-10868-to-stable/8.0
git checkout -b backport-10868-to-stable/8.0
ancref=$(git merge-base b8e2293ee4a7f0901aacb2bb19f32c3a00ff10ab 9d30d71bcee847176265c91124166385fc5e4143)
git cherry-pick -x $ancref..9d30d71bcee847176265c91124166385fc5e4143

@backport-action
Copy link
Collaborator

Successfully created backport PR #10870 for stable/8.1.

zeebe-bors-camunda bot added a commit that referenced this pull request Oct 31, 2022
10870: [Backport stable/8.1] Increase timeout and add logging r=npepinpe a=backport-action

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

relates to #10234

Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
zeebe-bors-camunda bot added a commit that referenced this pull request Oct 31, 2022
10870: [Backport stable/8.1] Increase timeout and add logging r=npepinpe a=backport-action

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

relates to #10234

Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
@korthout korthout added the version:8.1.3 Marks an issue as being completely or in parts released in 8.1.3 label Nov 1, 2022
zeebe-bors-camunda bot added a commit that referenced this pull request Nov 2, 2022
10871: [Backport stable/8.0] Increase timeout and add logging r=oleschoenburg a=npepinpe

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

relates to #10234

# Conflicts

There was a minor conflict in `ClusteringRule` around the logging added, mostly due to the changes we did when we moved the construction of `AtomixCluster` into the dist module. Nothing major, just add to move a line.

The PR also backports the `ClusteringRuleExtension`, as it will be useful in the future for more backports, I think.

Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version:8.1.3 Marks an issue as being completely or in parts released in 8.1.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants