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

Implement PausableScheduledThreadPool #61

Merged
merged 8 commits into from
Feb 3, 2021
Merged

Conversation

Linary
Copy link
Contributor

@Linary Linary commented Jan 21, 2021

No description provided.

@codecov
Copy link

codecov bot commented Jan 22, 2021

Codecov Report

Merging #61 (350a65f) into master (f4e861b) will decrease coverage by 0.00%.
The diff coverage is 90.62%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #61      +/-   ##
============================================
- Coverage     84.69%   84.69%   -0.01%     
- Complexity      746      756      +10     
============================================
  Files            58       59       +1     
  Lines          2150     2182      +32     
  Branches        320      321       +1     
============================================
+ Hits           1821     1848      +27     
- Misses          187      190       +3     
- Partials        142      144       +2     
Impacted Files Coverage Δ Complexity Δ
...egraph/concurrent/PausableScheduledThreadPool.java 88.88% <88.88%> (ø) 9.00 <9.00> (?)
...in/java/com/baidu/hugegraph/util/ExecutorUtil.java 87.50% <100.00%> (+5.68%) 5.00 <2.00> (+2.00)
...c/main/java/com/baidu/hugegraph/util/DateUtil.java 76.00% <0.00%> (-8.00%) 9.00% <0.00%> (-1.00%)

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 f4e861b...350a65f. Read the comment docs.

this.paused = false;
this.notifyAll();
synchronized (this) {
Copy link
Contributor

Choose a reason for hiding this comment

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

move this.paused = false; into synchronized block to avoid race between resumeSchedule and resumeSchedule

protected synchronized void beforeExecute(Thread t, Runnable r) {
if (this.paused) {
protected void beforeExecute(Thread t, Runnable r) {
synchronized (this) {
Copy link
Contributor

Choose a reason for hiding this comment

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

keep all paused writing synchronized

javeme
javeme previously approved these changes Jan 26, 2021
houzhizhen
houzhizhen previously approved these changes Jan 26, 2021
zhoney
zhoney previously approved these changes Jan 30, 2021
@Linary Linary dismissed stale reviews from zhoney, houzhizhen, and javeme via 618678a January 30, 2021 11:46
javeme
javeme previously approved these changes Feb 1, 2021
System.out.println("counter: " + counter.incrementAndGet());
}, 2, 2, TimeUnit.SECONDS);

Thread.sleep(4500);
Copy link
Contributor

Choose a reason for hiding this comment

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

4500L

AtomicInteger counter = new AtomicInteger(0);
executor.scheduleWithFixedDelay(() -> {
System.out.println("counter: " + counter.incrementAndGet());
}, 2, 2, TimeUnit.SECONDS);
Copy link
Contributor

Choose a reason for hiding this comment

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

2, 2 -> 2L, 2L


// pause
executor.pauseSchedule();
Thread.sleep(2000);
Copy link
Contributor

Choose a reason for hiding this comment

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

2000L


// resume
executor.resumeSchedule();
Thread.sleep(2000);
Copy link
Contributor

Choose a reason for hiding this comment

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

2000L

AtomicInteger counter = new AtomicInteger(0);
executor.scheduleAtFixedRate(() -> {
System.out.println("counter: " + counter.incrementAndGet());
}, 2, 2, TimeUnit.SECONDS);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

executor.scheduleAtFixedRate(() -> {
System.out.println("counter: " + counter.incrementAndGet());
}, 2, 2, TimeUnit.SECONDS);
Thread.sleep(4500);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto


// pause
executor.pauseSchedule();
Thread.sleep(2000);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto


// resume
executor.resumeSchedule();
Thread.sleep(2000);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

executor.pauseSchedule();

executor.shutdownNow();
executor.awaitTermination(3, TimeUnit.SECONDS);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

houzhizhen
houzhizhen previously approved these changes Feb 1, 2021
zhoney
zhoney previously approved these changes Feb 1, 2021
@Linary Linary dismissed stale reviews from zhoney and houzhizhen via 55864c3 February 2, 2021 06:03
@Linary Linary force-pushed the pausable-schedule-threadpool branch from 55864c3 to 978b157 Compare February 2, 2021 06:09
zhoney
zhoney previously approved these changes Feb 2, 2021
@javeme
Copy link
Contributor

javeme commented Feb 2, 2021

ci error
image

javeme
javeme previously approved these changes Feb 3, 2021
LicenseVerifyParamTest.class,
MachineInfoTest.class,
LicenseManagerTest.class,
// LockManagerTest.class,
Copy link
Contributor

Choose a reason for hiding this comment

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

keep origin

@javeme javeme merged commit c80f959 into master Feb 3, 2021
@javeme javeme deleted the pausable-schedule-threadpool branch February 3, 2021 14:03
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.

4 participants