search

openresty / test-nginx

Data-driven test scaffold for Nginx C module and OpenResty Lua library development
http://search.cpan.org/dist/Test-Nginx
438 stars 105 forks source link

bugfix: increased resiliency in HUP reload mode. #91

Closed thibaultcha closed 5 years ago

thibaultcha commented 5 years ago

This addresses two issues, both related to the HUP reload mode used with low TEST_NGINX_SLEEP values.

  1. Failing to ensure that old workers were properly rotated out from time to time.
  2. With the Valgrind mode enabled, failing to wait long enough for the init_by_lua phase to be run.

Two fixes improve resiliency in HUP testing mode:

  1. Increase the number of required config ver fetch successes to 20 for a HUP reload to be considered completed.
  2. Add an additional 0.2s waiting time in Valgrind mode.

This fixes test suites such as ngx_lua's t/151-initby-hup.t and lua-resty-core's t/shdict.t (and stream counterpart).

In all of my observations, the additional sleeping time from --- reload_fails block is not necessary anymore. In fact, only one test suite in the entirety of the OpenResty GitHub organization makes use of this section (t/151-initby-hup.t).

Example of related failures can be found in the t/151-initby-hup.t suite of ngx_lua in trv- mode (effectively, trvh- since the suite forces the HUP mode). Such errors can regularly be observed:

Failed test 'TEST 5: error in init after HUP, not reloaded but foo have changed. - response_body - response is expected (repeated req 0, req 0)'
   at /path/to/test-nginx/lib/Test/Nginx/Socket.pm line 1594.
          got: "hello, FOO\x{0a}"
       length: 11
     expected: "foo have changed\x{0a}"
       length: 17
     strings begin to differ at char 1 (line 1 column 1)

Because TEST 5's init_by_lua phase was supposed to update the shm foo value to "foo have changed", but was not given enough time to run before the test starts.