Improve HTTP caching for issue discussions
What does this MR do and why?
This skips rendering of notes to JSON and returns a 304 early when the notes have not changed.
Even before this change, we already return 304s because Rack::ETag
does this automatically by computing a digest from the response body and using that as the ETag. The problem with that is the whole response still needs to be computed so we don't save any server processing time but we save on network transfer time because we'll be returning an empty body.
With this change, we explicitly pass in the discussion cache keys so that they will be used to compute the ETag and we would skip the JSON serialization part. This leverages the discussion cache keys introduced by #332967 (closed) but this just does HTTP caching while MRs cache discussions in Redis.
There are pros and cons to both approaches and this will be a way to find out which one we should use or if we should use both. Because they're not really mutually exclusive.
How to set up and validate locally
- Enable the
issue_discussions_http_cache
feature flag. - Open an issue.
- Subsequent requests for
discussion.json
should return a 304 unless something is modified.
I tested this out locally on an issue with 500 notes.
Logs before this change
{"method":"GET","path":"/flightjs/flight_moved/-/issues/35/discussions.json","format":"json","controller":"Projects::IssuesController","action":"discussions","status":200,"time":"2021-10-19T06:27:43.275Z","params":[{"key":"namespace_id","value":"flightjs"},{"key":"project_id","value":"flight_moved"},{"key":"id","value":"35"}],"remote_ip":"192.168.68.103","user_id":1,"username":"root","ua":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:93.0) Gecko/20100101 Firefox/93.0","correlation_id":"01FJBKQV0NW782FH523TADYQF8","meta.user":"root","meta.project":"flightjs/flight_moved","meta.root_namespace":"flightjs","meta.caller_id":"Projects::IssuesController#discussions","meta.remote_ip":"192.168.68.103","meta.feature_category":"issue_tracking","meta.client_id":"user/1","redis_calls":8,"redis_duration_s":0.00216,"redis_read_bytes":451,"redis_write_bytes":84023,"redis_cache_calls":2,"redis_cache_duration_s":0.000742,"redis_cache_read_bytes":205,"redis_cache_write_bytes":83075,"redis_shared_state_calls":6,"redis_shared_state_duration_s":0.001418,"redis_shared_state_read_bytes":246,"redis_shared_state_write_bytes":948,"db_count":32,"db_write_count":0,"db_cached_count":5,"db_replica_count":32,"db_replica_cached_count":5,"db_replica_wal_count":0,"db_replica_wal_cached_count":0,"db_primary_count":0,"db_primary_cached_count":0,"db_primary_wal_count":0,"db_primary_wal_cached_count":0,"db_replica_duration_s":0.079,"db_primary_duration_s":0.0,"cpu_s":3.285327,"mem_objects":1893655,"mem_bytes":192587464,"mem_mallocs":816495,"mem_total_bytes":268333664,"pid":63,"queue_duration_s":1.475065,"db_duration_s":0.01876,"view_duration_s":0.14936,"duration_s":2.63021}
{"method":"GET","path":"/flightjs/flight_moved/-/issues/35/discussions.json","format":"json","controller":"Projects::IssuesController","action":"discussions","status":200,"time":"2021-10-19T06:29:05.518Z","params":[{"key":"namespace_id","value":"flightjs"},{"key":"project_id","value":"flight_moved"},{"key":"id","value":"35"}],"remote_ip":"192.168.68.103","user_id":1,"username":"root","ua":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:93.0) Gecko/20100101 Firefox/93.0","correlation_id":"01FJBKTC8DT9QX5PG8JD5QNSCW","meta.user":"root","meta.project":"flightjs/flight_moved","meta.root_namespace":"flightjs","meta.caller_id":"Projects::IssuesController#discussions","meta.remote_ip":"192.168.68.103","meta.feature_category":"issue_tracking","meta.client_id":"user/1","redis_calls":9,"redis_duration_s":0.002439,"redis_read_bytes":653,"redis_write_bytes":85775,"redis_cache_calls":3,"redis_cache_duration_s":0.001077,"redis_cache_read_bytes":407,"redis_cache_write_bytes":84830,"redis_shared_state_calls":6,"redis_shared_state_duration_s":0.001362,"redis_shared_state_read_bytes":246,"redis_shared_state_write_bytes":945,"db_count":32,"db_write_count":0,"db_cached_count":5,"db_replica_count":32,"db_replica_cached_count":5,"db_replica_wal_count":0,"db_replica_wal_cached_count":0,"db_primary_count":0,"db_primary_cached_count":0,"db_primary_wal_count":0,"db_primary_wal_cached_count":0,"db_replica_duration_s":0.072,"db_primary_duration_s":0.0,"cpu_s":2.88176,"mem_objects":1895331,"mem_bytes":196813656,"mem_mallocs":814506,"mem_total_bytes":272626896,"pid":64,"queue_duration_s":0.686475,"db_duration_s":0.01654,"view_duration_s":0.12787,"duration_s":2.4715}
{"method":"GET","path":"/flightjs/flight_moved/-/issues/35/discussions.json","format":"json","controller":"Projects::IssuesController","action":"discussions","status":200,"time":"2021-10-19T06:31:10.686Z","params":[{"key":"namespace_id","value":"flightjs"},{"key":"project_id","value":"flight_moved"},{"key":"id","value":"35"}],"remote_ip":"192.168.68.103","user_id":1,"username":"root","ua":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:93.0) Gecko/20100101 Firefox/93.0","correlation_id":"01FJBKY6HZM1B9E0R1SDS6DTJ5","meta.user":"root","meta.project":"flightjs/flight_moved","meta.root_namespace":"flightjs","meta.caller_id":"Projects::IssuesController#discussions","meta.remote_ip":"192.168.68.103","meta.feature_category":"issue_tracking","meta.client_id":"user/1","redis_calls":8,"redis_duration_s":0.002385,"redis_read_bytes":451,"redis_write_bytes":84014,"redis_cache_calls":2,"redis_cache_duration_s":0.000788,"redis_cache_read_bytes":205,"redis_cache_write_bytes":83069,"redis_shared_state_calls":6,"redis_shared_state_duration_s":0.001597,"redis_shared_state_read_bytes":246,"redis_shared_state_write_bytes":945,"db_count":32,"db_write_count":0,"db_cached_count":5,"db_replica_count":32,"db_replica_cached_count":5,"db_replica_wal_count":0,"db_replica_wal_cached_count":0,"db_primary_count":0,"db_primary_cached_count":0,"db_primary_wal_count":0,"db_primary_wal_cached_count":0,"db_replica_duration_s":0.075,"db_primary_duration_s":0.0,"cpu_s":2.987833,"mem_objects":1893664,"mem_bytes":191166472,"mem_mallocs":797473,"mem_total_bytes":266913032,"pid":63,"queue_duration_s":0.174229,"db_duration_s":0.01754,"view_duration_s":0.12758,"duration_s":2.91563}
Logs after this change
{"method":"GET","path":"/flightjs/flight_moved/-/issues/35/discussions.json","format":"json","controller":"Projects::IssuesController","action":"discussions","status":304,"time":"2021-10-19T06:21:39.194Z","params":[{"key":"namespace_id","value":"flightjs"},{"key":"project_id","value":"flight_moved"},{"key":"id","value":"35"}],"remote_ip":"192.168.68.103","user_id":1,"username":"root","ua":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:93.0) Gecko/20100101 Firefox/93.0","correlation_id":"01FJBKCS172YHVTRBTBA4DGAY1","meta.user":"root","meta.project":"flightjs/flight_moved","meta.root_namespace":"flightjs","meta.caller_id":"Projects::IssuesController#discussions","meta.remote_ip":"192.168.68.103","meta.feature_category":"issue_tracking","meta.client_id":"user/1","redis_calls":12,"redis_duration_s":0.0050290000000000005,"redis_read_bytes":1676,"redis_write_bytes":78571,"redis_cache_calls":6,"redis_cache_duration_s":0.002284,"redis_cache_read_bytes":1430,"redis_cache_write_bytes":77622,"redis_shared_state_calls":6,"redis_shared_state_duration_s":0.002745,"redis_shared_state_read_bytes":246,"redis_shared_state_write_bytes":949,"db_count":28,"db_write_count":0,"db_cached_count":3,"db_replica_count":28,"db_replica_cached_count":3,"db_replica_wal_count":0,"db_replica_wal_cached_count":0,"db_primary_count":0,"db_primary_cached_count":0,"db_primary_wal_count":0,"db_primary_wal_cached_count":0,"db_replica_duration_s":0.091,"db_primary_duration_s":0.0,"cpu_s":2.093836,"mem_objects":653618,"mem_bytes":119452184,"mem_mallocs":510321,"mem_total_bytes":145596904,"pid":63,"queue_duration_s":1.534067,"db_duration_s":0.02592,"view_duration_s":0.0,"duration_s":0.96758}
{"method":"GET","path":"/flightjs/flight_moved/-/issues/35/discussions.json","format":"json","controller":"Projects::IssuesController","action":"discussions","status":304,"time":"2021-10-19T06:22:20.906Z","params":[{"key":"namespace_id","value":"flightjs"},{"key":"project_id","value":"flight_moved"},{"key":"id","value":"35"}],"remote_ip":"192.168.68.103","user_id":1,"username":"root","ua":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:93.0) Gecko/20100101 Firefox/93.0","correlation_id":"01FJBKE1JQKCFS33BGDZ83J440","meta.user":"root","meta.project":"flightjs/flight_moved","meta.root_namespace":"flightjs","meta.caller_id":"Projects::IssuesController#discussions","meta.remote_ip":"192.168.68.103","meta.feature_category":"issue_tracking","meta.client_id":"user/1","redis_calls":7,"redis_duration_s":0.001931,"redis_read_bytes":248,"redis_write_bytes":70335,"redis_cache_calls":1,"redis_cache_duration_s":0.000357,"redis_cache_read_bytes":2,"redis_cache_write_bytes":69392,"redis_shared_state_calls":6,"redis_shared_state_duration_s":0.001574,"redis_shared_state_read_bytes":246,"redis_shared_state_write_bytes":943,"db_count":27,"db_write_count":0,"db_cached_count":3,"db_replica_count":27,"db_replica_cached_count":3,"db_replica_wal_count":0,"db_replica_wal_cached_count":0,"db_primary_count":0,"db_primary_cached_count":0,"db_primary_wal_count":0,"db_primary_wal_cached_count":0,"db_replica_duration_s":0.064,"db_primary_duration_s":0.0,"cpu_s":1.625618,"mem_objects":645190,"mem_bytes":118166888,"mem_mallocs":509794,"mem_total_bytes":143974488,"pid":63,"queue_duration_s":1.801715,"db_duration_s":0.01576,"view_duration_s":0.0,"duration_s":0.89308}
{"method":"GET","path":"/flightjs/flight_moved/-/issues/35/discussions.json","format":"json","controller":"Projects::IssuesController","action":"discussions","status":304,"time":"2021-10-19T06:23:36.190Z","params":[{"key":"namespace_id","value":"flightjs"},{"key":"project_id","value":"flight_moved"},{"key":"id","value":"35"}],"remote_ip":"192.168.68.103","user_id":1,"username":"root","ua":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:93.0) Gecko/20100101 Firefox/93.0","correlation_id":"01FJBKGB1DN5EDNSQGQKSFK1GE","meta.user":"root","meta.project":"flightjs/flight_moved","meta.root_namespace":"flightjs","meta.caller_id":"Projects::IssuesController#discussions","meta.remote_ip":"192.168.68.103","meta.feature_category":"issue_tracking","meta.client_id":"user/1","redis_calls":8,"redis_duration_s":0.090257,"redis_read_bytes":462,"redis_write_bytes":71468,"redis_cache_calls":2,"redis_cache_duration_s":0.001461,"redis_cache_read_bytes":216,"redis_cache_write_bytes":70519,"redis_shared_state_calls":6,"redis_shared_state_duration_s":0.088796,"redis_shared_state_read_bytes":246,"redis_shared_state_write_bytes":949,"db_count":27,"db_write_count":0,"db_cached_count":3,"db_replica_count":27,"db_replica_cached_count":3,"db_replica_wal_count":0,"db_replica_wal_cached_count":0,"db_primary_count":0,"db_primary_cached_count":0,"db_primary_wal_count":0,"db_primary_wal_cached_count":0,"db_replica_duration_s":0.077,"db_primary_duration_s":0.0,"cpu_s":1.74552,"mem_objects":646403,"mem_bytes":117628616,"mem_mallocs":506146,"mem_total_bytes":143484736,"pid":64,"queue_duration_s":1.828617,"db_duration_s":0.01605,"view_duration_s":0.0,"duration_s":0.92867}
You can see the reduced duration_s
and mem_*
numbers.
Some notes:
- Before this change (when
Rack::Etag
converts the 200 into 304s), thestatus
that is logged is200
even though304
is actually sent and received by the browser. I think this is becauseRack::Etag
is executed after the logging.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.