Dylan Hardison [:dylan] (he/him) Assignee
	Description • 7 years ago

Expected Behavior:

Bugzilla->request_cache has the lifecycle of one request.
At the end of the request, we clear it.

Observed Behavior:

Bugzilla->request_cache has contents of it before each page request,
and these contents are the same every web worker.

Resolution:

This was puzzling, leading me to believe the request cache wasn't being cleared,
or the params hash or something weirder. It turns out the problem is that so long as Bugzilla->request_cache() is called during apache startup, every worker spawned after that will inherit its contents up to that point.

The correct thing to do here would be to clear the request cache
at the start of each worker, or before each request.
The former would be more ideal but is probably not compatible with plack/psgi which we hope to use some day.

	Dylan Hardison [:dylan] (he/him) Assignee
	Comment 1 • 7 years ago

I should also add this was only a problem for staging/dev as their apache size limits are set too low.
Each worker is killed after every request, so they're always starting out with a fresh (dirty) request cache.

	Dylan Hardison [:dylan] (he/him) Assignee
	Comment 2 • 7 years ago

1. clear request cache after all cgis are compiled.
2. clear request cache before each request
3. move cleanup handler before SizeLimit.

NI: gozer, handlers get executed in the order listed, correct?

	:glob ✱
	Comment 3 • 7 years ago

(In reply to Dylan Hardison [:dylan] from comment #2)
> NI: gozer, handlers get executed in the order listed, correct?

yes

https://perl.apache.org/docs/2.0/user/handlers/intro.html#Stacked_Handlers
PerlCleanupHandler -> RUN_ALL
"Handlers of the type RUN_ALL will be executed in the order they have been registered.."

	:glob ✱
	Comment 4 • 7 years ago

Comment on attachment 8847762 [details] [diff] [review]
1347335_1.patch

Review of attachment 8847762 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob

::: mod_perl.pl
@@ +116,4 @@
>      $rl->handler($file, $file);
>  }
>  
> +Bugzilla::clear_request_cache();

a brief comment explaining why this is necessary would be useful

	Philippe M. Chiasson (:gozer)
	Comment 5 • 7 years ago

(In reply to Dylan Hardison [:dylan] from comment #0)
> Expected Behavior:
> 
> Bugzilla->request_cache has the lifecycle of one request.
> At the end of the request, we clear it.
> 
> Observed Behavior:
> 
> Bugzilla->request_cache has contents of it before each page request,
> and these contents are the same every web worker.
> 
> Resolution:
> 
> This was puzzling, leading me to believe the request cache wasn't being
> cleared,
> or the params hash or something weirder. It turns out the problem is that so
> long as Bugzilla->request_cache() is called during apache startup, every
> worker spawned after that will inherit its contents up to that point.

The joy of copy-on-write, this makes complete sense.

> I should also add this was only a problem for staging/dev as their apache size limits are set too low.
> Each worker is killed after every request, so they're always starting out with a fresh (dirty) request cache.

As a side-node, having Apache::SizeLimit kill workers after every request in prod is somewhat troubling... That amounts to CGI execution mode with no real benefits from caching...

> [...]
> NI: gozer, handlers get executed in the order listed, correct?

Correct!

> 1. clear request cache after all cgis are compiled.

Or disable caching during startup.

> 2. clear request cache before each request

Hoping it's not performance heavy, but a good safety/cleanliness measure

> 3. move cleanup handler before SizeLimit.

Yup.

	Dylan Hardison [:dylan] (he/him) Assignee
	Comment 6 • 7 years ago

(In reply to Philippe M. Chiasson (:gozer) from comment #5)
> As a side-node, having Apache::SizeLimit kill workers after every request in
> prod is somewhat troubling... That amounts to CGI execution mode with no
> real benefits from caching...

not in prod, in staging/dev. But also fixed now.

	Dylan Hardison [:dylan] (he/him) Assignee
	Comment 7 • 7 years ago

With comments.

To git@github.com:mozilla-bteam/bmo.git
   badbd77..7a16fee  master -> master