Application limit for ES indexed field length
What does this MR do?
Using the plan_limit
table we can down store a limit for the maximum size of any fields being indexed in Elasticsearch. This is for #201826 (closed) . Any strings above the length limit will be truncated down to the limit.
We default to 0
which means unlimited so this has no affect for self-managed customers. On GitLab.com we will likely want to set this to around 20k as a sensible upper limit on what we should store.
Screenshots
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Merge request reports
Activity
added Application Limits Category:Global Search advanced search bugperformance devopssystems groupglobal search ~13003331 typefeature labels
changed milestone to %12.8
added 1 commit
- 2cb5ef52 - Application limit for ES indexed field length
marked the checklist item Changelog entry as completed
marked the checklist item Documentation (if required) as completed
- Resolved by Dylan Griffith
- Resolved by Dylan Griffith
@dgruzd would you mind giving this a review.
@rdickenson would you mind reviewing the docs here.
assigned to @dgruzd and @rdickenson
added database databasereview pending labels
1 Message This merge request adds or changes files that require a review from the Database team. This merge request requires a database review. To make sure these changes are reviewed, take the following steps:
- Ensure the merge request has database and databasereview pending labels. If the merge request modifies database files, Danger will do this for you.
- Prepare your MR for database review according to the docs.
- Assign and mention the database reviewer suggested by Reviewer Roulette.
The following files require a review from the Database team:
db/migrate/20200204070729_add_elasticsearch_indexed_field_length_limit_to_application_settings.rb
db/schema.rb
Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has randomly picked a candidate for each review slot. Feel free to override this selection if you think someone else would be better-suited, or the chosen person is unavailable.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines.
Once you've decided who will review this merge request, mention them as you normally would! Danger does not (yet?) automatically notify them for you.
Category Reviewer Maintainer database Steve Abrams ( @sabrams
)Toon Claes ( @toon
)backend Ross Fuhrman ( @rossfuhrman
)Thong Kuah ( @tkuah
)frontend Jay Swain ( @jayswain
)Natalia Tepluhina ( @ntepluhina
)test Quality for spec/features/*
Grant Young ( @grantyoung
)No maintainer available Generated by
DangerEdited by 🤖 GitLab Bot 🤖unassigned @dgruzd
added 1 commit
- 2d178b81 - Application limit for ES indexed field length
mentioned in issue #201805 (closed)
- Resolved by Dylan Griffith
- Resolved by Steve Abrams
- Resolved by Steve Abrams
@DylanGriffith just one comment on the database side of things.
assigned to @DylanGriffith and unassigned @sabrams
mentioned in issue gitlab-com/www-gitlab-com#5028 (closed)
assigned to @sabrams and unassigned @DylanGriffith, @tkuah, and @rdickenson
added databasereviewed label and removed databasereview pending label
assigned to @tkuah and @rdickenson
- Resolved by Mayra Cabrera
- Resolved by Thong Kuah
Thanks @DylanGriffith - the specs look great - just a question about how plans work.
assigned to @DylanGriffith and unassigned @tkuah
assigned to @tkuah and unassigned @DylanGriffith, @toon, and @rdickenson
assigned to @rdickenson and @toon
- Resolved by Russell Dickenson
added 1 commit
- 9bde964f - Application limit for ES indexed field length
assigned to @mayra-cabrera and unassigned @toon
- Resolved by Mayra Cabrera
@mayra-cabrera would you mind doing a database review here. Note we discussed the migration in !24345 (comment 282283991)
- Resolved by Mayra Cabrera
- Resolved by Mayra Cabrera
- Resolved by Mayra Cabrera
unassigned @mayra-cabrera
@DylanGriffith Thanks, please see my response above.
assigned to @DylanGriffith and unassigned @tkuah and @rdickenson
added 1 commit
- d31b23c1 - Application limit for ES indexed field length
added 647 commits
-
d31b23c1...ad9bec95 - 646 commits from branch
master
- e2e8cec9 - Application limit for ES indexed field length
-
d31b23c1...ad9bec95 - 646 commits from branch
- Resolved by Thong Kuah
@mayra-cabrera @tkuah thanks for your comments. I've made the suggested changes.
assigned to @rdickenson, @mayra-cabrera, and @tkuah
unassigned @DylanGriffith
added 1 commit
- d1c3ecd5 - Application limit for ES indexed field length
- Resolved by Dylan Griffith
- Resolved by Dylan Griffith
added databaseapproved label and removed databasereviewed label
assigned to @DylanGriffith and unassigned @mayra-cabrera
- Resolved by Dylan Griffith
- Resolved by Dylan Griffith
- Resolved by Dylan Griffith
Thanks @DylanGriffith - some minor questions otherwise I'm happy to approve!
unassigned @tkuah and @rdickenson
added 1 commit
- 2c2c823d - Application limit for ES indexed field length
@tkuah back to you now with a couple of changes.
assigned to @tkuah and unassigned @DylanGriffith
LGTM! Thanks @DylanGriffith
enabled an automatic merge when the pipeline for 2c2c823d succeeds
@DylanGriffith - Looks like https://gitlab.com/gitlab-org/gitlab/-/jobs/431089259 is a real failure. The other two IIRC are
master
failures, since fixed. Perhaps a rebase will resolve thoseassigned to @DylanGriffith and unassigned @tkuah
added 1 commit
- a3aae17f - Application limit for ES indexed field length
@tkuah thanks for spotting that. Yes I forgot to add the translations. I've also rebased now.
assigned to @tkuah and unassigned @DylanGriffith
added 177 commits
-
a3aae17f...2e33d3e6 - 176 commits from branch
master
- d76b4c3b - Application limit for ES indexed field length
-
a3aae17f...2e33d3e6 - 176 commits from branch
marked the checklist item Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. as completed
enabled an automatic merge when the pipeline for d76b4c3b succeeds
mentioned in commit e767eb5b
mentioned in merge request !29728 (merged)
mentioned in merge request !36925 (merged)
added devopsdata stores label and removed devopssystems label