XComp commented on code in PR #24309:
URL: https://github.com/apache/flink/pull/24309#discussion_r1499457151


##########
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/slotmanager/FineGrainedSlotManager.java:
##########
@@ -243,7 +253,7 @@ private void registerSlotManagerMetrics() {
     /** Suspends the component. This clears the internal state of the slot 
manager. */
     @Override
     public void suspend() {

Review Comment:
   Yes, I missed the assert here.



##########
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/slotmanager/FineGrainedSlotManager.java:
##########
@@ -108,27 +110,30 @@ public class FineGrainedSlotManager implements 
SlotManager {
     /** ResourceManager's id. */
     @Nullable private ResourceManagerId resourceManagerId;
 
-    /** Executor for future callbacks which have to be "synchronized". */
-    @Nullable private Executor mainThreadExecutor;
+    private final Object lock = new Object();
+    /**
+     * Executor for future callbacks which have to be "synchronized". This 
field being {@code null}
+     * indicates that the component isn't started.
+     */
+    @GuardedBy("lock")
+    @Nullable
+    private volatile ComponentMainThreadExecutor mainThreadExecutor;

Review Comment:
   correct, the volatile doesn't make sense here. I have to protected the 
`mainThreadExecutor` with a lock because of the `runInMainThreadIfStarted` 
method checks for the main thread executor and executes the Runnable within the 
lock.



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

Reply via email to