Github user liyichao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18084#discussion_r118424700
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/master/Master.scala 
---
    @@ -796,9 +796,12 @@ private[deploy] class Master(
       }
     
       private def relaunchDriver(driver: DriverInfo) {
    -    driver.worker = None
    -    driver.state = DriverState.RELAUNCHING
    -    waitingDrivers += driver
    +    removeDriver(driver.id, DriverState.RELAUNCHING, None)
    +    val newDriver = createDriver(driver.desc)
    --- End diff --
    
    First, we must distinguish the original driver and the newly relaunched 
one, because there will be statusUpdate of the two versions to arrive at 
master. For example, when the network partitioned worker reconnects to master, 
it will send `DriverStateChanged` with the driver id, and master must recognize 
it is the state of the original driver and the newly launched driver.
    
    The patch simply choose a new driver id to do this, which also has some 
Shortcomings, however. For example, In the UI, the two versions of driver are 
not related, and the final state is `RELAUNCHING`(which seems better to be 
relaunched).  
    
    Another way is to add some like `attemptId` to driver state, and then Let 
`DriverStateChanged` bring the attemptId to indicate its entity. This seems 
more complex.
    
    What's your opinion? 
    
    It seems hard something like `attemptId` to the persistent driver state?   
Looking forward to your opinions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to