xintongsong commented on code in PR #24541: URL: https://github.com/apache/flink/pull/24541#discussion_r1593914163
########## flink-datastream-api/src/main/java/org/apache/flink/datastream/api/context/NonPartitionedRuntimeContext.java: ########## @@ -0,0 +1,27 @@ +/* + * 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.flink.datastream.api.context; + +import org.apache.flink.annotation.Experimental; + +/** + * A {@link NonPartitionedRuntimeContext} contains all execution information unrelated to partition. + */ +@Experimental +public interface NonPartitionedRuntimeContext {} Review Comment: The contexts become confusing. There are currently 5 contexts, and there relationship and differences are unclear. - RuntimeContext - PartitionedRuntimeContext - NonPartitionedRuntimeContext - NonPartitionedContext - TwoOutputNonPartitionedContext I think the information / functionalities provided by these contexts can be categorized into 3 kinds - Only needed in partitioned context (PartitionedRuntimeContext) - Only needed in non-partitioned context (NonPartitionedContext & TwoOutputNonPartitionedContext) - Needed by both partitioned and non partitioned (NonPartitionedRuntimeContext) Therefore, I'd suggest: - RuntimeContext for things needed by both partitioned and non-partitioned, which is exactly the current NonPartitionedRuntimeContext - OneOutputNonPartitionedContext and TwoOutputNonPartitionedContext, both extends RuntimeContext, for things only needed in non-partitioned context, which are the current NonPartitionedContext & TwoOutputNonPartitionedContext - PartitionedContext, extends RuntimeContext, for things only needed in partitioned context, which is the current RuntimeContext - The current PartitionedRuntimeContext is no longer needed, as is is neither directly implemented by any concrete classes, nor extended by multiple sub-interfaces/classes. ########## flink-core-api/src/main/java/org/apache/flink/api/common/operators/SlotSharingGroupDescriptor.java: ########## @@ -0,0 +1,238 @@ +/* + * 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.flink.api.common.operators; + +import org.apache.flink.annotation.Experimental; +import org.apache.flink.configuration.MemorySize; + +import javax.annotation.Nullable; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; + +/** + * The descriptor that describe the name and the different resource components of a slot sharing + * group. + */ +@Experimental +public class SlotSharingGroupDescriptor { Review Comment: Not sure about the name `xxxDescriptor`. Maybe we can keep the name but move it to another package for v2. Since we are no longer exposing the concept of operator, maybe just move it to the new package `o.a.f.api.common`. And please make sure we document the purpose of having two SSG classes and their differences clearly in JavaDocs. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
