* processes any pending records which are not migrated yet
* bumps import_export version because of new commits_count attribute
* removes commits_count fallback method
* processes any pending records which are not migrated yet
* bumps import_export version because of new commits_count attribute
* removes commits_count fallback method
For each MR diff an extra 'SELECT COUNT()' is executed
to get number of commits for the diff. Overall time to get counts for
all MR diffs may be quite expensive. To speed up loading of MR info,
information about number of commits is stored in a MR diff's extra column.
Closes#38068
The Gitaly CommitService is being hammered by n + 1 calls, mostly when
finding commits. This leads to this gRPC being turned of on production:
https://gitlab.com/gitlab-org/gitaly/issues/514#note_48991378
Hunting down where it came from, most of them were due to
MergeRequest#show. To prove this, I set a script to request the
MergeRequest#show page 50 times. The GDK was being scraped by
Prometheus, where we have metrics on controller#action and their Gitaly
calls performed. On both occations I've restarted the full GDK so all
caches had to be rebuild.
Current master, 806a68a81f, needed 435 requests
After this commit, 154 requests
The st_commits and st_diffs columns on merge_request_diffs historically held the
YAML-serialised data for a merge request diff, in a variety of formats.
Since 9.5, these have been migrated in the background to two new tables:
merge_request_diff_commits and merge_request_diff_files. That has the advantage
that we can actually query the data (for instance, to find out how many commits
we've stored), and that it can't be in a variety of formats, but must match the
new schema.
This is the final step of that journey, where we drop those columns and remove
all references to them. This is a breaking change to the importer, because we
can no longer import diffs created in the old format, and we cannot guarantee
the export will be in the new format unless it was generated after this commit.
Compared to the merge_request_diff association:
1. It's simpler to query. The query uses a foreign key to the
merge_request_diffs table, so no ordering is necessary.
2. It's faster for preloading. The merge_request_diff association has to load
every diff for the MRs in the set, then discard all but the most recent for
each. This association means that Rails can just query for N diffs from N
MRs.
3. It's more complicated to update. This is a bidirectional foreign key, so we
need to update two tables when adding a diff record. This also means we need
to handle this as a special case when importing a GitLab project.
There is some juggling with this association in the merge request model:
* `MergeRequest#latest_merge_request_diff` is _always_ the latest diff.
* `MergeRequest#merge_request_diff` reuses
`MergeRequest#latest_merge_request_diff` unless:
* Arguments are passed. These are typically to force-reload the association.
* It doesn't exist. That means we might be trying to implicitly create a
diff. This only seems to happen in specs.
* The association is already loaded. This is important for the reasons
explained in the comment, which I'll reiterate here: if we a) load a
non-latest diff, then b) get its `merge_request`, then c) get that MR's
`merge_request_diff`, we should get the diff we loaded in c), even though
that's not the latest diff.
Basically, `MergeRequest#merge_request_diff` is the latest diff in most cases,
but not quite all.
Once we migrate existing MRs to have this column, we will be able to get the
latest diff for a single merge request more efficiently, and (more importantly)
get all latest diffs for a collection of MRs efficiently.
MergeRequest#create_merge_request_diff and MergeRequest#reload_diff are
the only places where we generate a new MR diff so that's where we
should fetch the ref.
This also ensures that the ref is not fetched when we call
merge_request.merge_request_diffs.create in
Github::Import#fetch_pull_requests.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Also, improved a bit the method names in
Github::Representation::PullRequest.
Last but not least, ensure temp branch names doesn't contain a `/` as
this would create the ref in a subfolder in `refs/heads` (e.g.
`refs/heads/gh-123/456/rymai/foo`), and would leave empty directories
upon branch deletion (e.g. `refs/heads/gh-123/456/rymai/`.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Previously, we stored these as serialised fields - `st_{commits,diffs}` - on the
`merge_request_diffs` table. These now have their own tables -
`merge_request_diff_{commits,diffs}` - with a column for each attribute of the
serialised data.
Add a background migration to go through the existing MR diffs and migrate them
to the new format. Ignore any contents that cannot be displayed. Assuming that
we have 5 million rows to migrate, and each batch of 2,500 rows can be
completed in 5 minutes, this will take about 7 days to migrate everything.
Previously, we used Psych, which would:
1. Check if a string was encoded as binary, and not ASCII-compatible.
2. Add the !binary tag in that case.
3. Convert to base64.
We need to do the same thing, using a new column in place of the tag.
This is an ID-less table with just three columns: an association to the merge
request diff the commit belongs to, the relative order of the commit within the
merge request diff, and the commit SHA itself.
Previously we stored much more information about the commits, so that we could
display them even when they were deleted from the repo. Since 8.0, we ensure
that those commits are kept around for as long as the target repo itself is, so
we don't need to duplicate that data in the database.
This adds an ID-less table containing one row per file, per merge request
diff. It has a column for each attribute on Gitlab::Git::Diff that is serialised
currently, with the advantage that we can easily query the attributes of this
new table.
It does not migrate existing data, so we have fallback code when the legacy
st_diffs column is present instead. For a merge request diff to be valid, it
should have at most one of:
* Rows in this new table, with the correct merge_request_diff_id.
* A non-NULL st_diffs column.
It may have neither, if the diff is empty.
The problem is that we often go via a diff object constructed from the diffs
stored in the DB. Those diffs, by definition, don't overflow, so we don't have
access to the 'correct' `real_size` - that is stored on the MR diff object
iself.
* upstream/master: (1122 commits)
Update CHANGELOG.md for 8.16.2
Display project ID in project settings (!8572)
fixed points from comments to improve code quality
Update CHANGELOG.md for 8.14.8
Statisfy eslint
Add CHANGELOG entry
Fix access to the wiki code via HTTP when repository feature disabled
Display fullscreen button on small screens (!5302)
Prevent removing fields from dropdowns on input elements
fix for all themes
Return struct instead of multiple values
Fix race conditions for AuthorizedProjectsWorker
Add User#nested_groups and User#nested_projects methods
Fix spec failure due to timestamp ordering issue in mySQL
Fixed error with filter keyboard tests
`can?` already includes the `feature_available?` check
Test there is no Merge Request button when MRs are disabled
Ensure the correct Merge Request button is found
Add 409 conflict tests
Add CHANGELOG
...
Michael Kozono
Rémy Coutable
Oswaldo Ferreira
gfyoung
Jan Provaznik
Ahmad Sherif
Hiroyuki Sato
Zeger-Jan van de Weg
Sean McGivern
Yorick Peterse
James Lopez
Alejandro Rodríguez
Bob Van Landuyt
Robert Speicher
Douwe Maan
James Lopez
Lin Jen-Shin
Sean McGivern
Felipe Artur
Robert Speicher