baiyangtx commented on code in PR #3233: URL: https://github.com/apache/amoro/pull/3233#discussion_r1793585658
########## amoro-common/src/main/java/org/apache/amoro/ActionStage.java: ########## @@ -0,0 +1,46 @@ +/* + * 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.amoro; + +public class ActionStage { + + private final String desc; + private final int weight; + + /** + * Action Stage description + * + * @param desc Action Stage description value, normally this value should be identical within + * certain actions + * @param weight the weight number of this action, the bigger the weight number, the higher + * position on front pages + */ + public ActionStage(String desc, int weight) { + this.desc = desc; + this.weight = weight; + } Review Comment: Stage information needs to be persisted together with Task, so Stage needs to have a field for database persistence. Of course, order can also be used for persistence. ########## amoro-common/src/main/java/org/apache/amoro/ActionStage.java: ########## @@ -0,0 +1,46 @@ +/* + * 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.amoro; + +public class ActionStage { + + private final String desc; + private final int weight; + + /** + * Action Stage description + * + * @param desc Action Stage description value, normally this value should be identical within + * certain actions + * @param weight the weight number of this action, the bigger the weight number, the higher + * position on front pages + */ Review Comment: The front-end pages are sorted by Process and Action weight. So the weight here defines the order in which a group of Tasks are executed in Process, so I think it is more reasonable to call it order. ########## amoro-common/src/main/java/org/apache/amoro/process/TableProcessState.java: ########## @@ -53,7 +53,7 @@ public long getId() { } public String getName() { - return action.getDescription(); Review Comment: Action must be annotated with @StateField, otherwise Process recovery cannot be performed ########## amoro-common/src/main/java/org/apache/amoro/ActionStage.java: ########## @@ -0,0 +1,46 @@ +/* + * 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.amoro; + +public class ActionStage { + + private final String desc; + private final int weight; + + /** + * Action Stage description + * + * @param desc Action Stage description value, normally this value should be identical within + * certain actions + * @param weight the weight number of this action, the bigger the weight number, the higher + * position on front pages + */ + public ActionStage(String desc, int weight) { + this.desc = desc; + this.weight = weight; + } Review Comment: Spark uses an incremented stage id to identify a stage and for fault recovery. We can refer to its implementation. ########## amoro-common/src/main/java/org/apache/amoro/ActionStage.java: ########## @@ -0,0 +1,46 @@ +/* + * 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.amoro; + +public class ActionStage { Review Comment: Is the concept of ProcessStage more accessible. ########## amoro-common/src/main/java/org/apache/amoro/Action.java: ########## @@ -18,62 +18,43 @@ package org.apache.amoro; -import org.apache.amoro.process.TableProcess; -import org.apache.amoro.process.TableProcessState; -import org.apache.amoro.shade.guava32.com.google.common.collect.Sets; +public class Action { -import java.util.Collections; -import java.util.Set; - -public enum Action { - MINOR_OPTIMIZING("minor-optimizing", 0), - MAJOR_OPTIMIZING("major-optimizing", 1), - EXTERNAL_OPTIMIZING("external-optimizing", 2), - // refresh all metadata including snapshots, watermark, configurations, schema, etc. - REFRESH_METADATA("refresh-metadata", 10), - // expire all metadata and data files necessarily. - EXPIRE_DATA("expire-data", 11), - DELETE_ORPHAN_FILES("delete-orphan-files", 12), - SYNC_HIVE_COMMIT("sync-hive-commit", 13); + private final TableFormat[] formats; + private final int code; + private final int weight; + private final String desc; /** - * Arbitrary actions are actions that can be handled by a single optimizer. The processes they - * related to like refreshing, expiring, cleaning and syncing all share the same basic - * implementations which are {@link TableProcess} and {@link TableProcessState} and they won't - * have any spitted stages like optimizing processes(plan, execute, commit), so they can be easily - * triggered and managed. If you want to add a new action which is handled stand-alone, you should - * add it to this set, and you would find it's easy to implement the process and state. + * Create a new action + * + * @param formats supported table formats of this action + * @param code storage code of this action, normally this code should be identical within + * supported formats + * @param weight the weight number of this action, the bigger the weight number, the higher + * positions of schedulers or front pages + * @param desc description of this action, will be shown in front pages */ - public static final Set<Action> ARBITRARY_ACTIONS = - Collections.unmodifiableSet( - Sets.newHashSet(REFRESH_METADATA, EXPIRE_DATA, DELETE_ORPHAN_FILES, SYNC_HIVE_COMMIT)); - - public static boolean isArbitrary(Action action) { - return ARBITRARY_ACTIONS.contains(action); + Action(TableFormat[] formats, int code, int weight, String desc) { Review Comment: It is recommended that the constructor be private and can only be created through a static `register` method, and that CODE needs to be checked globally for conflicts when register a new ACTION. Refer to: TableForamt https://github.com/apache/amoro/blob/master/amoro-common/src/main/java/org/apache/amoro/TableFormat.java -- 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]
