jiangfucheng commented on code in PR #9909:
URL: https://github.com/apache/apisix/pull/9909#discussion_r1287081976


##########
t/node/healthcheck-stop-checker.t:
##########
@@ -256,7 +247,6 @@ ok
 --- grep_error_log eval
 qr/create new checker: table: 0x|try to release checker: table: 0x/
 --- grep_error_log_out
-try to release checker: table: 0x

Review Comment:
   This line can be remove, the reasons are as follows:
   
   1.`healthchekcer` will be remove after worker exit since the status store in 
memroy, that's why `TEST 5` will not print `try to release checker: table: 0x` 
in first line.
   2.Why the `try to release checker: table: 0x` will be print if we add 
`sleep(1)` before worker exited
       - Because the log is printed by old worker. Before old worker be killed, 
the new worker will be created, at this moment, both old worker and new worker 
can receive events, it's easy to be proved through print debug log before 
execute `fire_all_clean_handlers`
      
   ```
               if pre_index then
               local pre_val = self.values[pre_index]
               log.info("sync_data: check pre_val: ", 
inspect(pre_val.clean_handlers), worker_id_str, " pid: ", ngx.worker.pid())
               if pre_val then
                   config_util.fire_all_clean_handlers(pre_val)
               end
   ```
    logs:
    
   ```
   2023/08/08 18:04:31 [info] 77582#1004031: *312 [lua] config_etcd.lua:741: 
sync_data(): sync_data: check pre_val: { {
   f = <function 1>,
   id = 1
   },
     _id = 2
   } worker_id: 0 pid: 77582, context: ngx.timer
   
   2023/08/08 18:04:31 [info] 77620#1004305: *447 [lua] config_etcd.lua:741: 
sync_data(): sync_data: check pre_val: {} worker_id: 0 pid: 77620, context: 
ngx.timer
   ```
   
   We can see the etcd events be received with two `wokrer 0`, and there pid is 
different, we can easliy to confirm the worker is old worker which has 
`check_handlers` field
   
   3.Why these test cases can passed in old version(before 
https://github.com/apache/apisix/issues/9456 be merged)
       - Because in the old version, the worker is not exit immediately too, it 
will exit after `make quit/reload` about 60s, so the reason is same as above.



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