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

    https://github.com/apache/flink/pull/6216#discussion_r199738568
  
    --- Diff: 
flink-end-to-end-tests/test-scripts/test_queryable_state_restart_tm.sh ---
    @@ -85,20 +90,23 @@ function run_test() {
             exit 1
         fi
     
    -    local 
current_num_checkpoints=current_num_checkpoints$(get_completed_number_of_checkpoints
 ${JOB_ID})
    -
         kill_random_taskmanager
     
         latest_snapshot_count=$(cat $FLINK_DIR/log/*out* | grep "on snapshot" 
| tail -n 1 | awk '{print $4}')
         echo "Latest snapshot count was ${latest_snapshot_count}"
     
    -    sleep 65 # this is a little longer than the heartbeat timeout so that 
the TM is gone
    +    # wait until the TM loss was detected
    +    wait_for_job_state_transition ${JOB_ID} "RESTARTING" "CREATED"
     
         start_and_wait_for_tm
     
    +    wait_job_running ${JOB_ID}
    +
    +    local current_num_checkpoints="$(get_completed_number_of_checkpoints 
${JOB_ID})"
    --- End diff --
    
    So when I ran this test locally it frequently occurred that we never 
actually waited for checkpoints to complete; the number of checkpoint that we 
waited for had already occurred and we exited early.
    The job just switches to running, and right away we got 3 completed 
checkpoints?
    This was a bit, well, _odd_. 
    
    In the previous version the checkpoint count could further increase while 
we shut down the TM.
    Technically there's no guarantee how up-to-date the result if 
`get_completed_number_of_checkpoints` or how long `kill_random_taskmanager` 
actually takes, so we may have fulfilled the checkpoint count condition before 
the job even restarts. It's admittedly unlikely.
    
    This change simply guarantees that the job actually completes N checkpoints 
after it was restarted, and just serves to eliminate doubts about the 
correctness of the test.


---

Reply via email to