From 11dfad3e3a3209532184d828c1f70e6b774ade71 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 20 Jan 2017 15:13:27 -0800 Subject: Add performance/scalability concerns to CONTRIBUTING.md [ci skip] --- CONTRIBUTING.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1fd29fef4f0..9a6e3feec4c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -399,6 +399,12 @@ There are a few rules to get your merge request accepted: 1. Contains functionality we think other users will benefit from too 1. Doesn't add configuration options or settings options since they complicate making and testing future changes +1. Changes do not adversely degrade performance. + - Avoid repeated polling of endpoints that require a significant amount of overhead + - Check for N+1 queries via the SQL log or [`QueryRecorder`](https://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) + - Avoid repeated access of filesystem +1. If you need polling to support real-time features, consider using this [described long + polling approach](https://gitlab.com/gitlab-org/gitlab-ce/issues/26926). 1. Changes after submitting the merge request should be in separate commits (no squashing). If necessary, you will be asked to squash when the review is over, before merging. @@ -434,6 +440,7 @@ the feature you contribute through all of these steps. 1. Description explaining the relevancy (see following item) 1. Working and clean code that is commented where needed 1. Unit and integration tests that pass on the CI server +1. Performance/scalability implications have been considered, addressed, and tested 1. [Documented][doc-styleguide] in the /doc directory 1. Changelog entry added 1. Reviewed and any concerns are addressed -- cgit v1.2.1 From 84561349ffa7aa079f5bd371ba51bef02ee8f6df Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Tue, 14 Mar 2017 14:45:38 +0100 Subject: Describe polling with ETag caching --- CONTRIBUTING.md | 5 +++-- doc/development/polling.md | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 doc/development/polling.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9a6e3feec4c..ae143c58290 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -403,8 +403,8 @@ There are a few rules to get your merge request accepted: - Avoid repeated polling of endpoints that require a significant amount of overhead - Check for N+1 queries via the SQL log or [`QueryRecorder`](https://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - Avoid repeated access of filesystem -1. If you need polling to support real-time features, consider using this [described long - polling approach](https://gitlab.com/gitlab-org/gitlab-ce/issues/26926). +1. If you need polling to support real-time features, please use + [polling with ETag caching][polling-etag]. 1. Changes after submitting the merge request should be in separate commits (no squashing). If necessary, you will be asked to squash when the review is over, before merging. @@ -547,6 +547,7 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor [UX Guide for GitLab]: http://docs.gitlab.com/ce/development/ux_guide/ [license-finder-doc]: doc/development/licensing.md [GitLab Inc engineering workflow]: https://about.gitlab.com/handbook/engineering/workflow/#labelling-issues +[polling-etag]: https://docs.gitlab.com/ce/development/polling.html [^1]: Specs other than JavaScript specs are considered backend code. Haml changes are considered backend code if they include Ruby code other than just diff --git a/doc/development/polling.md b/doc/development/polling.md new file mode 100644 index 00000000000..a086aca6697 --- /dev/null +++ b/doc/development/polling.md @@ -0,0 +1,41 @@ +# Polling with ETag caching + +Polling for changes (repeatedly asking server if there are any new changes) +introduces high load on a GitLab instance, because it usually requires +executing at least a few SQL queries. This makes scaling large GitLab +instances (like GitLab.com) very difficult so we do not allow adding new +features that require polling and hit the database. + +Instead you should use polling mechanism with ETag caching in Redis. + +## How to use it + +1. Add the path of the endpoint which you want to poll to + `Gitlab::EtagCaching::Middleware`. +1. Implement cache invalidation for the path of your endpoint using + `Gitlab::EtagCaching::Store`. Whenever a resource changes you + have to invalidate the ETag for the path that depends on this + resource. +1. Check that the mechanism works: + - requests should return status code 304 + - there should be no SQL queries logged in `log/development.log` + +## How it works + +1. Whenever a resource changes we generate a random value and store it in + Redis. +1. When a client makes a request we set the `ETag` response header to the value + from Redis. +1. The client caches the response (client-side caching) and sends the ETag as + the `If-None-Modified` header with every subsequent request for the same + resource. +1. If the `If-None-Modified` header matches the current value in Redis we know + that the resource did not change so we can send 304 response immediately, + without querying the database at all. The client's browser will use the + cached response. +1. If the `If-None-Modified` header does not match the current value in Redis + we have to generate a new response, because the resource changed. + +For more information see: +- [RFC 7232](https://tools.ietf.org/html/rfc7232) +- [ETag proposal](https://gitlab.com/gitlab-org/gitlab-ce/issues/26926) -- cgit v1.2.1