azagrebin commented on a change in pull request #13321:
URL: https://github.com/apache/flink/pull/13321#discussion_r485441451
##########
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:
ok, it is a pity that `JobVertex` is mutable and we do not have a
separate builder for `JobVertex`.
is this `if` basically not the same as just initialising `slotSharingGroup =
new SlotSharingGroup()` in the constructor?
this should be also inline with the previous behaviour where all vertexes
with null-group get into a separate group
----------------------------------------------------------------
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
