Disable DB writes if node is a GitLab Geo secondary instance

Added 613 commits:

4d39dbcb...49f9f633 - 612 commits from branch master
f6fe5422 - Disable DB writes if node is a GitLab Geo secondary instance

Added 1 commit:

5331c59d - Avoid DB setup failures by checking if Geo is secondary within HTML cache methods

In general, our overall read-only database handling with Geo needs improvement. We may need to consider something like Octupus or Makara.

/cc: @patricio, @brodock

Added ~164929 label

@nick.thomas Can I have you take over this MR and decide whether this is an acceptable patch for Geo customers?

@stanhu this is almost there. However there is also a site in lib/banzai/renderer.rb,which may cause a database write from a HTTP GET request:

    42      def render_field(object, field)
    43        html_field = object.markdown_cache_field_for(field)
    44
    45        html = object.__send__(html_field)
    46        return html if html.present?
    47
    48        html = cacheless_render_field(object, field)
    49        object.update_column(html_field, html) unless object.new_record? || object.destroyed?
    50
    51        html
    52      end

This also needs a fix. On it now.

Reassigned to @nick.thomas

Added 1 commit:

0f5ec763 - Disable DB writes if node is a GitLab Geo secondary instance

Compare with previous version

Unmarked this merge request as a Work In Progress

Added 1 commit:

582926b7 - Disable DB writes if node is a GitLab Geo secondary instance

Compare with previous version

Reassigned to @stanhu

Added regression label

Can we have a switchable class responsible for caching this data? Something we can change during "initialize" from "cache to the database" to "cache into redis".

The "interface" should have a "cache", and at least a "expire" (key) / "clear" (all)

@brodock that would rely on us being able to invalidate the database cache after some time using the redis cache, and vice-versa. It's a bit thorny.

@nick.thomas Could you take over this MR as you see fit?

Reassigned to @nick.thomas

Sorry for the amount of time this is taking @stanhu. The interception point has been merged into CE now, so nearly there.

@nick.thomas the idea is that any secondary geo instance cannot use database cache (because it's a readonly node), so it has to use redis for that.

@brodock can you pop it into a separate issue? I'd rather focus on getting it working again at-all in this one, and I don't see a good invalidation strategy that would lead to us displaying the correct data in all cases at the moment.

Mentioned in issue #1217 (closed)

Added 647 commits:

582926b7...7117b4b9 - 502 commits from branch master
624e3d91 - add reminder to check paths in nginx config
64e2d884 - Return conflict error in label API when title is taken by group label
5416d0e0 - Pass @ref along so we know which pipeline to show
17dfcf6d - Add a test for showing correct pipeline
7fac0e17 - Add CHANGELOG entry
13c1859a - Merge remote-tracking branch 'upstream/master' into show-status-from-branch
51a012b8 - Be more specific, feedback:
7cdb238a - Merge remote-tracking branch 'upstream/master' into show-status-from-branch
b4ef158a - Still show status from pipelines, see:
6f6a8d2a - Also update for default project page, feedback:
141cb35d - Also pass ref here
fb19d9f4 - Use multi-line conditions in view, feedback:
f4b9de38 - It's not used as a public API right now, feedback:
3e6a527e - Add tests for Commit#status and Commit#status_for, feedback:
e036abf3 - Always use multiline in view
e5c6f943 - Backport Group API code that was added in EE only
0fa15505 - Update deckar01-task_list gem to 1.0.6
7c84037e - Added Element extension with .matches and .closest
bdf7365d - Hides registry if project is disabled
fb3229d7 - CHANGELOG item
a6ba358a - Make specs a bit faster
80b5ab5d - Merge remote-tracking branch 'upstream/master' into show-status-from-branch
acfe3940 - Add PoC for resource serializers
f1e9c97d - Use entity request object in environment entity
573921cb - Implement entities needed for environments folders
52d333b9 - Remove duplication from build serializer entity
7dd6485a - Add manual actions for deployment serialization
c3dae798 - Extend tests for environments serializer
6c94adea - Remove project entity that was part of the PoC
a68e7e60 - Add basic tests for build entity used by serialzier
3c3e61aa - Exclude project information from environment entity
f203ca5c - Fix specs related to deployments and environments
ee4c8b75 - Fix CHANGELOG
1ae557c1 - Merge status_for and status, feedback:
0902ba52 - Initialize @statuses in status rather than constructor
f3c3d8e6 - There's no such method
4ba789cd - Remove unused gitlab:generate_docs Rake task
9dc3c4c0 - Update rdoc to ~> 4.2
d84eb9ab - Issues atom feed url reflect filters on dashboard
46641888 - Update avatar container name so it doesnt conflict with other image containers
2cddbeee - Unify dropdown button styles
5bfbb446 - Add custom text to column
ca1a64c0 - Remove underline focus state on statuses
813fd6f6 - DRY up the specs for bin/changelog
2a1d701b - Add new icon for skipped builds; show created state in mini graph
2471b01b - Remove g from svg colors
6a00cf1b - Merge remote-tracking branch 'upstream/master' into show-status-from-branch
ce1dc4c2 - Update for CHANGELOG
31245ea3 - Merge branch 'rs-changelog-spec-dry' into 'master'
bc5837c0 - Merge branch 'build-status-focus-state' into 'master'
cc83039a - Merge branch '24133-custom-signin-text-alignment' into 'master'
e900e53a - Show log corresponding to env in admin/logs
8a998b63 - Change a bunch of doc links to either relative or https://docs.gitlab.com.
055e2ac2 - Move jquery.timeago to vendor directory
18dd4cf5 - Update match-regex to fix filename convention
ac1bbe7a - Merge branch 'file-toggle' into 'master'
257075e4 - Merge branch 'project-container-repo-disable' into 'master'
630c40b9 - Merge branch '22914-mini-graph-icons' into 'master'
6e1802d0 - Add jquery.timeago.js to application.js
99a000da - Add CHANGELOG entry file
3a8a7c12 - Merge branch 'fix-invalid-filename-eslint' into 'master'
06dcb077 - Add tip for using Chrome to run and debug teaspoon tests.
251a20a3 - Split out markdown cache storage into a separate method
b1b51b77 - Cleaned up global namespace JS
c1388d0e - Allow multiple repository storage shards to be enabled, and automatically round-robin between them
fa4f904f - Document multiple repository storage paths
a3847fa0 - Add changelog entry
b27de4ab - Ignore builds directory from eslint
b0f89867 - Merge branch 'ignore-build-dir-eslint' into 'master'
a04a451f - Merge branch 'patch-9' into 'master'
957744da - Merge branch 'gitlab-ce-cleanup-global-namespace-javascript' into 'master'
27528d44 - Move spinach public snippet feature to rspec spec
a2c1178c - Expose commit author if author exists
c315332b - Refine build entity tests a little
2cb7285b - change build list height to show 6,5 builds + improve padding of list, with firs…
b708c0d3 - updated styling commit SHA on branches page + added to changelog
7f6607e3 - Update commits.scss
277712cc - Creates new list from a new label
1a08981f - Merge branch '24017-long-build-list-dropdowns-which-scroll-are-not-immediately-o…
90dfae93 - Merge branch '23979-show-environment-log' into 'master'
a30d34b2 - Merge branch 'spec_optimization' into 'master'
7f320142 - GrapeDSL for project hooks
64f41179 - Move shared params to a helper
66bcf875 - Merge branch '24022-update-styling-commit-sha-in-branches-list' into 'master'
73ab9ba5 - Merge branch '22947-fix_issues_atom_feed_url' into 'master'
dcd70453 - Fix: Guest sees some repository details and gets 404
e2768498 - Merge branch 'fix-docs-links' into 'master'
69bff037 - Merge branch 'clarify-nginx-installation' into 'master'
5ef2bd19 - Merge branch '24059-round-robin-repository-storage' into 'master'
b7dff1b0 - Fix: Todos Filter Shows All Users
5f1d3ec5 - Only skip group when it's actually a group in the "Share with group" select
3c4a7f91 - Add width to caret to rid ourselves of the shift
3968b07d - Add tests for serialization entities, add user entity
856ef3c3 - Merge branch '24056-guest-sees-some-project-details-and-gets-404' into 'master'
e49fb264 - Add tests for deployment and environment entitites
588bc001 - Merge branch 'backport-ee-js-groups-api' into 'master'
e4c05de7 - Merge branch 'rs-update-rdoc' into 'master'
6d6fb561 - added various changes to commit title area
30d4cce0 - Update commit.scss to have correct linting
ff337964 - Update commit page styles
a3cc2f1e - Merge branch '23961-can-t-share-project-with-groups' into 'master'
050c8dc7 - Apply *_params_ce + *_params_ee pattern to MergeRequestsController
1db9f826 - Add setting to only allow merge requests to be merged when all discussions are resolved
3f029144 - Complete and improve specs
065ba130 - Add documentation for the "Only allow merge requests to be merged if all discuss…
6a9fb20f - Fix applying GitHub-imported labels when importing job is interrupted
eaa0303b - Cache imported GH labels no matter what
b0a4635b - Simplify implementation of entity serializers
b2bbac88 - Merge branch '22588-todos-filter-shows-all-users' into 'master'
8c2143a0 - Allow to test JIRA service when project does not have repository
d7210329 - Update GitLab Workhorse to v1.0.0
5368b9f2 - Merge branch 'zj-grapify-project-hooks' into 'master'
fb9a0e52 - Merge remote-tracking branch 'upstream/master' into show-status-from-branch
b9b508ad - Remove author according to the document
99410a47 - Fetch locals to avoid undefined method/local error
d12e7647 - Merge branch '20968-add-setting-to-check-unresolved-discussion' into 'master'
e517da3f - Created wait_for_turbolinks and added test for refs dropdown selection with special chars
920551b5 - Merge branch 'rs-merge_request_params' into 'master'
d38dd3c4 - Merge remote-tracking branch 'origin/labels-api'
5b220b38 - Merge branch 'master' of dev.gitlab.org:gitlab/gitlabhq
5a0a4506 - Merge branch 'feature/view-related-serializers' into 'master'
6eb77d9e - Merge branch 'update-task-list' into 'master'
5d0f68f2 - Fix diff_comments failure
86b8fb4e - Merge branch 'show-status-from-branch' into 'master'
8d0d8b91 - Merge branch 'fix/gh-imported-labels-are-not-applied-correctly' into 'master'
7cb3fbbb - Correct spelling of Andreesen's name
d0b28cd0 - Merge branch 'issue_23032' into 'master'
dd5f0762 - Merge branch 'stylistic-changes-to-commit-title-bar' into 'master'
cccc6df5 - Merge branch 'fixed-width-caret' into 'master'
6f78b264 - Merge branch 'issue-board-new-label-new-list' into 'master'
1dfe10d3 - Merge branch 'add-matches-and-closest-element-extensions' into 'master'
14419ef5 - Merge branch 'gitlab-workhorse-1.0.0' into 'master'
b3dc990d - Merge branch 'add-special-char-tests-for-refs-dropdown' into 'master'
09f4af04 - Merge branch 'remove-wiki-image-border-radius' into 'master'
50f3fd49 - Merge branch 'move-public-snippets-feature-to-rspec' into 'master'
a0907921 - Remove logging to console in diff_comments_store_spec
ab17b094 - Merge branch 'correct-spelling-andreesen' into 'master'
9a1ded1f - Merge branch 'split-out-markdown-cache-update' into 'master'
c392b0cc - Merge branch 'remove-diff-comments-spec-logging' into 'master'
95eeaa99 - Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into ce_upstream
b1e3a13c - resolve conflict in app/views/projects/_merge_request_settings.html.haml
adec6248 - Resolved application.js conflicts
8504e278 - fix conflict resolving
3e8f41fb - Application.js confirm modal conflict fix
97e39069 - Disable DB writes if node is a GitLab Geo secondary instance

Compare with previous version

OK, I've rebased this MR against ce_upstream since that has https://gitlab.com/gitlab-org/gitlab-ee/commit/9a1ded1fe6bb615a9b92b7b463d099700245cf98 in it already.

@yorickpeterse happy? :) once ce_upstream is merged it should look sensible.

Is it too late for this to be in 8.13.4?

Resolved all discussions

@nick.thomas Is 145 commits correct? That seems like a large amount, are you sure the rebase went OK? Perhaps the target branch is not correct?

@yorickpeterse https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/850 is 144 commits and not merged yet, plus the one commit in here, so that seems right to me :)

Marked this merge request as a Work In Progress

Added 19 commits:

97e39069...88b03a4c - 18 commits from branch master
ee60ec8d - Disable DB writes if node is a GitLab Geo secondary instance

Compare with previous version

Unmarked this merge request as a Work In Progress

OK, rebased against master and all tests are green. @yorickpeterse happy to merge?

Reassigned to @yorickpeterse

Status changed to merged

Mentioned in commit 88e88a80

Mentioned in issue #1243 (closed)

Picked into 8-13-stable-ee, will go into 8.13.6.

Removed ~149424 label

Mentioned in commit 9c9de16f

Admin message

Admin message

Disable DB writes if node is a GitLab Geo secondary instance

Merge request reports

Activity