zhuzhurk commented on a change in pull request #13321:
URL: https://github.com/apache/flink/pull/13321#discussion_r485449187



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobVertex.java
##########
@@ -364,25 +365,29 @@ public void 
addOperatorCoordinator(SerializedValue<OperatorCoordinator.Provider>
         * @param grp The slot sharing group to associate the vertex with.
         */
        public void setSlotSharingGroup(SlotSharingGroup grp) {
+               checkNotNull(grp);
+
                if (this.slotSharingGroup != null) {
                        
this.slotSharingGroup.removeVertexFromGroup(this.getID(), 
this.getMinResources());
                }
 
+               grp.addVertexToGroup(this.getID(), this.getMinResources());
                this.slotSharingGroup = grp;
-               if (grp != null) {
-                       grp.addVertexToGroup(this.getID(), 
this.getMinResources());
-               }
        }
 
        /**
         * Gets the slot sharing group that this vertex is associated with. 
Different vertices in the same
-        * slot sharing group can run one subtask each in the same slot. If the 
vertex is not associated with
-        * a slot sharing group, this method returns {@code null}.
+        * slot sharing group can run one subtask each in the same slot.
         *
-        * @return The slot sharing group to associate the vertex with, or 
{@code null}, if not associated with one.
+        * @return The slot sharing group to associate the vertex with
         */
-       @Nullable
        public SlotSharingGroup getSlotSharingGroup() {
+               if (slotSharingGroup == null) {
+                       // create a new slot sharing group for this vertex if 
it was in no other slot sharing group.
+                       // this should only happen in testing cases at the 
moment because production code path will
+                       // always set a value to it before used

Review comment:
       It's not the same because we need to invoke `setSlotSharingGroup()` 
which relies on `getMinResources` of the vertex.
   The `getMinResources`, however, is set after the vertex is constructed.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to