Skip to content

clear environment variables before calling hooks

What does this MR do?

Clear environment variables other than the ones specified explicitly before calling external ruby programs (i.e. hooks like before-update and before-receive which are implemented in ruby in Gitlab Shell), saving about 55% of the time. The time saved are probably the time used by rubygems and GEM_HOME environment variable to locate proper gem locations. Benchmark results follow,

Before Optimization

15 reqeusts, 13097.80 ms in total

call_receive_hook (899.3ms)
call_update_hook (624.4ms)
call_receive_hook (845.6ms)
call_receive_hook (1373.6ms)
call_update_hook (639.1ms)
call_receive_hook (876.3ms)
call_receive_hook (1128.7ms)
call_update_hook (650.3ms)
call_receive_hook (851.9ms)
call_receive_hook (1023.9ms)
call_update_hook (651.0ms)
call_receive_hook (910.6ms)
call_receive_hook (991.1ms)
call_update_hook (691.3ms)
call_receive_hook (940.7ms)

After Optimization

15 requests 5974.20 ms in total

call_receive_hook (1938.4ms)
call_update_hook (33.2ms)
call_receive_hook (241.4ms)
call_receive_hook (274.5ms)
call_update_hook (31.8ms)
call_receive_hook (236.2ms)
call_receive_hook (1862.3ms)
call_update_hook (33.8ms)
call_receive_hook (233.3ms)
call_receive_hook (246.9ms)
call_update_hook (34.3ms)
call_receive_hook (237.8ms)
call_receive_hook (280.0ms)
call_update_hook (37.5ms)
call_receive_hook (252.8ms)

Are there points in the code the reviewer needs to double check?

Unsetting environment variables could have bad side effects. This is exactly like invoking ruby scripts in crontab without any environment set up. rvm will not work if the server depends on it. rubygems might not work since GEM_HOME environment variable is gone.

Why was this MR needed?

Every web edit is slow due to huge amount of time spent in calling hooks.

What are the relevant issue numbers?

https://gitlab.com/gitlab-org/gitlab-ce/issues/15452

@DouweM @yorickpeterse

Screenshots (if relevant)

N/A

Merge request reports

Loading