valepakh commented on code in PR #2894: URL: https://github.com/apache/ignite-3/pull/2894#discussion_r1412021915
########## modules/api/src/main/java/org/apache/ignite/compute/JobState.java: ########## @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.compute; + +/** + * Compute job's state enum. + */ +public enum JobState { + /** + * The job was submitted and added to the queue and waiting queue for execution. Review Comment: This comment is now not entirely valid - it's possible that we don't wait in queue at all. ########## modules/api/src/main/java/org/apache/ignite/compute/JobState.java: ########## @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.compute; + +/** + * Compute job's state enum. + */ +public enum JobState { + /** + * The job was submitted and added to the queue and waiting queue for execution. + */ + QUEUED, + + /** + * The job is being executed. + */ + EXECUTING, + + /** + * The job was unexpectedly terminated during execution. + */ + FAILED, + + /** + * The job was executed successfully and the execution result was returned. + */ + COMPLETED, + + /** + * Job has received the cancel command, but is still running. + */ + CANCELING, + + /** + * Job was successfully cancelled. Review Comment: ```suggestion * The job was successfully cancelled. ``` ########## modules/compute/src/main/java/org/apache/ignite/internal/compute/state/ComputeStateMachine.java: ########## @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.compute.state; + +import java.util.UUID; +import org.apache.ignite.compute.JobState; + +/** + * State machine of Compute Jobs. + */ +public interface ComputeStateMachine { + /** + * Initialize Compute job in state machine. This job should have status {@link JobState.QUEUED}. Review Comment: ```suggestion * Initialize Compute job in state machine. This job should have status {@link JobState#QUEUED}. ``` ########## modules/api/src/main/java/org/apache/ignite/compute/JobState.java: ########## @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.compute; + +/** + * Compute job's state enum. + */ +public enum JobState { + /** + * The job was submitted and added to the queue and waiting queue for execution. + */ + QUEUED, + + /** + * The job is being executed. + */ + EXECUTING, + + /** + * The job was unexpectedly terminated during execution. + */ + FAILED, + + /** + * The job was executed successfully and the execution result was returned. + */ + COMPLETED, + + /** + * Job has received the cancel command, but is still running. Review Comment: ```suggestion * The job has received the cancel command, but it is still running. ``` ########## modules/compute/src/main/java/org/apache/ignite/internal/compute/state/IllegalJobStateTransition.java: ########## @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.compute.state; + +import static org.apache.ignite.lang.ErrorGroups.Compute.COMPUTE_JOB_STATE_TRANSITION_ERR; + +import java.util.UUID; +import org.apache.ignite.compute.JobState; +import org.apache.ignite.internal.lang.IgniteInternalException; + +/** + * Thrown from Compute Jobs state machine {@link ComputeStateMachine} when job state transfer is illegal. + */ +public class IllegalJobStateTransition extends IgniteInternalException { + public IllegalJobStateTransition(UUID jobId) { + super(COMPUTE_JOB_STATE_TRANSITION_ERR, "Failed to transfer job state for nonexistent job" + jobId + "."); Review Comment: ```suggestion super(COMPUTE_JOB_STATE_TRANSITION_ERR, "Failed to transfer job state for nonexistent job " + jobId + "."); ``` ########## modules/compute/src/main/java/org/apache/ignite/internal/compute/state/ComputeStateMachine.java: ########## @@ -0,0 +1,73 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.compute.state; + +import java.util.UUID; +import org.apache.ignite.compute.JobState; + +/** + * State machine of Compute Jobs. + */ +public interface ComputeStateMachine { + /** + * Initialize Compute job in state machine. This job should have status {@link JobState.SUBMITTED}. + * + * @return Compute job identifier. + */ + UUID initJob(); + + /** + * Try to transfer Compute Job to complete state. + * + * @param jobId Compute job identifier. + * @throws IllegalJobStateTransition in case when job can't be transferred to complete state. + */ + void completeJob(UUID jobId); + + /** + * Try to transfer Compute Job to execute state. + * + * @param jobId Compute job identifier. + * @throws IllegalJobStateTransition in case when job can't be transferred to complete state. Review Comment: The error is still there - it should be `can't be transferred to the executing state`, or even `@{link JobState#EXECUTING} state` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
