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

Synchronous Execution of Astro Startup Jobs and minor fixes #3746

Merged
merged 1 commit into from Jun 26, 2017

Conversation

Projects
None yet
4 participants
@amitjoy
Copy link
Member

commented Jun 26, 2017

The scheduling operations schedules few jobs and executes 2 specific jobs immediately. These two jobs are executed in a different thread to prevent blocking the thread that spawns it.

In addition, during handler initialize, channel linking and unlinking, we reschedule the jobs. And every time, before we reschedule the jobs, we remove the jobs from the ESH Core scheduler that are associated with the current thing.

As the jobs that execute immediately are handled by different threads, it is sometime noticed that while rescheduling, removal of jobs did occur after the jobs got rescheduled. This created unexpected issues in ESH Core scheduler. In this PR, we have rendered the asynchronous operations of executing immediate jobs in different threads to synchronous execution so that we can make sure the removal always happens before rescheduling.

Another note is that previously we required equals and hashcode to be overriden by AbstractJob but currently it is not used anymore. These equals and hashcode affected normal executions of scheduled jobs as well. Hence, the methods have been removed.

@kaikreuzer

This comment has been minimized.

Copy link
Member

commented Jun 26, 2017

Could you add a short description to the PR about what it does and fixes exactly?

@amitjoy

This comment has been minimized.

Copy link
Member Author

commented Jun 26, 2017

@sjka

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2017

Thanks for this fix!

I can confirm that the equals/hashcode implementation indeed was messing up the internal calculations of the next execution time in the scheduler.

Doing the initial job scheduling synchronously also sounds like a pretty good idea to me.

@amitjoy could you please fix the commit message and additionally include a sign-off-by line?

@sjka

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2017

PS: fixes #3728
(note to self: auto-close won't work...)

@amitjoy

This comment has been minimized.

Copy link
Member Author

commented Jun 26, 2017

@sjka 😢 How did I forget it? Fixing it.

@sjka

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2017

All good, that's what we have all this auto-validation stuff for 😄

Synchronous Execution of Astro Startup Jobs and minor fixes
Signed-off-by: Amit Kumar Mondal <admin@amitinside.com>

@amitjoy amitjoy force-pushed the amitjoy:fix_missing_events branch from e069ed0 to eda0988 Jun 26, 2017

@triller-telekom
Copy link
Contributor

left a comment

LGTM

@kaikreuzer kaikreuzer merged commit 04a064c into eclipse:master Jun 26, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ip-validation
Details

@amitjoy amitjoy deleted the amitjoy:fix_missing_events branch Jun 26, 2017

@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017

@kaikreuzer kaikreuzer added the bug label Dec 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.