Look-Y-Y commented on PR #27211:
URL: https://github.com/apache/flink/pull/27211#issuecomment-3627858113

   > > > Thanks for your fix @Look-Y-Y . I have some suggestions: introducing 
`getRunningFuture` may make the semantic of the `delay `parameter in `schedule` 
unclear—it will become “**delay for some time after running**” instead of 
“**delay the time from now until execution**”. This could break some by-design 
behaviors.
   > > > I’d prefer either adding a `start()` function in 
`DefaultBlocklistHandler#scheduleTimeoutCheck` so that it’s invoked after the 
Endpoint starts, or introducing an `isRunning()` method to the gateway, so that 
we can log a warning when it’s not running.
   > > 
   > > 
   > > Thank for your suggestion @noorall . I plan to make the following 
modifications:
   > > 
   > > 1. Add a `start()` interface method to `BlocklistHandler`, and move the 
`mainThreadExecutor` field from the constructor to the `start()` method to 
ensure all operations are performed after `start()`.
   > > 2. Call the `BlocklistHandler#start()` method after 
`JobMaster/ResourceManager` starts the RPC endpoint.
   > > 3. Call the `RpcEndpoint#isRunning()` method in `MainThreadExecutor`. If 
the RPC is not running, log a warning.
   > 
   > Hi @Look-Y-Y , thanks again for working on this PR. Do you have any 
updates recently?
   
   Hello @noorall I have updated this PR. Points 1 and 2 above have been 
completed. Regarding point 3, since the scheduling method is not called by the 
main thread, not log RPC not running.


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

Reply via email to