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 exclusive job execution #1203

Merged
merged 14 commits into from Jan 23, 2019

Conversation

metanet
Copy link
Contributor

@metanet metanet commented Jan 17, 2019

There can be at most one active job (i.e., not completed/failed job)
with the same name. Once a named job completes, its name can be reused.

BREAKING CHANGE: When a new job with a duplicate name is submitted via newJob(), it throws an exception.

@metanet metanet added the core label Jan 17, 2019
@metanet metanet added this to the 0.8 milestone Jan 17, 2019
@devOpsHazelcast
Copy link
Contributor

Test FAILed.

@viliam-durina viliam-durina self-requested a review January 17, 2019 14:03
@devOpsHazelcast
Copy link
Contributor

Test FAILed.

@devOpsHazelcast
Copy link
Contributor

Test FAILed.

@devOpsHazelcast
Copy link
Contributor

Test PASSed.

1 similar comment
@devOpsHazelcast
Copy link
Contributor

Test PASSed.

@devOpsHazelcast
Copy link
Contributor

Test PASSed.

Copy link
Contributor

@cangencer cangencer left a comment

Choose a reason for hiding this comment

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

breaking changes must also be documented

@@ -188,6 +189,14 @@ public Job newJob(@Nonnull DAG dag, @Nonnull JobConfig config) {
return instance.newJob(dag, config);
}

@Nonnull @Override
public Job newJobIfAbsent(@Nonnull DAG dag, @Nonnull JobConfig config) {
if (jarPathname != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicated logic, should be extracted to method

*
* <p>See also {@link #newJobIfAbsent}.
*
* @throws DuplicateActiveJobNameException if there is an active job with
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe JobAlreadyExistsException would be better?

* limitations under the License.
*/

package com.hazelcast.jet.core;
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be in core but in .jet as it's a public facing exception

@devOpsHazelcast
Copy link
Contributor

Test PASSed.

@devOpsHazelcast
Copy link
Contributor

Test PASSed.

@devOpsHazelcast
Copy link
Contributor

Test PASSed.

1 similar comment
@devOpsHazelcast
Copy link
Contributor

Test PASSed.

@cangencer cangencer merged commit e5ce76a into hazelcast:master Jan 23, 2019
@devOpsHazelcast
Copy link
Contributor

Test PASSed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants