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 flaky test cases with a bit more robust waiting logic #8154

Merged
merged 2 commits into from
Feb 8, 2022

Conversation

klsince
Copy link
Contributor

@klsince klsince commented Feb 7, 2022

Description

fix flaky test cases with a bit more robust waiting logic, as the test cases wait for the ZK event to trigger the call backs.

Upgrade Notes

Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)

  • Yes (Please label as backward-incompat, and complete the section below on Release Notes)

Does this PR fix a zero-downtime upgrade introduced earlier?

  • Yes (Please label this as backward-incompat, and complete the section below on Release Notes)

Does this PR otherwise need attention when creating release notes? Things to consider:

  • New configuration options
  • Deprecation of configurations
  • Signature changes to public methods/interfaces
  • New plugins added or old plugins removed
  • Yes (Please label this PR as release-notes and complete the section on Release Notes)

Release Notes

Documentation

Copy link
Member

@richardstartin richardstartin left a comment

Choose a reason for hiding this comment

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

👍

// jobDetail and jobTrigger are not added atomically by the scheduler,
// the jobDetail is added to an internal map firstly, and jobTrigger
// is added to another internal map afterwards, so we check for the existence
// of jobTrigger with some waits to be more defensive.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

 org.quartz.simpl.RAMJobStore:

  public void storeJobAndTrigger(JobDetail newJob, OperableTrigger newTrigger) throws JobPersistenceException {
    this.storeJob(newJob, false);
    this.storeTrigger(newTrigger, false);
  }

@codecov-commenter
Copy link

codecov-commenter commented Feb 7, 2022

Codecov Report

Merging #8154 (a6883ae) into master (8bbf93a) will increase coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #8154      +/-   ##
============================================
+ Coverage     71.39%   71.47%   +0.08%     
  Complexity     4303     4303              
============================================
  Files          1624     1624              
  Lines         84198    84303     +105     
  Branches      12602    12635      +33     
============================================
+ Hits          60116    60259     +143     
+ Misses        19970    19919      -51     
- Partials       4112     4125      +13     
Flag Coverage Δ
integration1 28.93% <ø> (+0.06%) ⬆️
integration2 27.70% <ø> (+0.02%) ⬆️
unittests1 67.91% <ø> (-0.06%) ⬇️
unittests2 14.18% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ller/helix/core/minion/TaskTypeMetricsUpdater.java 80.00% <0.00%> (-20.00%) ⬇️
...ache/pinot/core/operator/docidsets/OrDocIdSet.java 86.36% <0.00%> (-11.37%) ⬇️
...he/pinot/segment/local/segment/store/IndexKey.java 70.00% <0.00%> (-5.00%) ⬇️
...ntroller/helix/core/minion/TaskMetricsEmitter.java 86.36% <0.00%> (-4.55%) ⬇️
...y/aggregation/function/SumAggregationFunction.java 95.45% <0.00%> (-4.55%) ⬇️
...y/aggregation/function/MaxAggregationFunction.java 96.36% <0.00%> (-3.64%) ⬇️
...y/aggregation/function/MinAggregationFunction.java 96.36% <0.00%> (-3.64%) ⬇️
...ator/transform/function/CaseTransformFunction.java 61.36% <0.00%> (-3.31%) ⬇️
...not/broker/broker/helix/ClusterChangeMediator.java 77.65% <0.00%> (-2.13%) ⬇️
...controller/helix/core/minion/PinotTaskManager.java 67.51% <0.00%> (-1.83%) ⬇️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8bbf93a...a6883ae. Read the comment docs.

@klsince
Copy link
Contributor Author

klsince commented Feb 7, 2022

any thoughts on the compability check failure below?

Error:  Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.1:install-node-and-npm (install node and npm) on project pinot-controller: Could not extract the Node archive: Could not extract archive: '/tmp/compatibility-verifier/mvn-compat-cache/1644273388/com/github/eirslett/node/10.16.1/node-10.16.1-linux-x64.tar.gz': EOFException -> [Help 1]

@klsince klsince force-pushed the fix_flaky_test_cases branch from 2f8ece0 to a6883ae Compare February 7, 2022 23:53
@xiangfu0 xiangfu0 merged commit a47af49 into apache:master Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants