Skip to content
Snippets Groups Projects

Increase the page size for the contributions GraphQL query

Merged Adam Hegyi requested to merge 431546-use-500-batch-size-for-ca into master
2 unresolved threads

What does this MR do and why?

This change increases the max page size for the contributions query. Clients may request up to 500 records per page. The main user of this query is the Contribution Analytics feature, where we noticed that large volume of batching (pagination) queries affect the page load significantly: https://gitlab.com/gitlab-com/gitlab-OKRs/-/work_items/4084#note_1644920882

By default, we preserve the default page size (100) for backward compability reason. The frontend controls the page size which is currently behind a feature flag: use_500_page_size_for_contribution_analytics

How to set up and validate locally

  1. Ensure that you're on premium.
  2. Go to a group and visit Analyze > Contribution Analytics
  3. Inspect the GraphQL request and notice that it uses first: null (means: fall back to the default page size.
  4. Enable the use_500_page_size_for_contribution_analytics feature flag.
  5. Not the same request should sent the first: 500 argument.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #431546 (closed)

Edited by Adam Hegyi

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Contributor

    Multiversion compatibility

    This merge request updates GraphQL backend and frontend code.

    To prevent an incident, ensure the updated frontend code is backwards compatible.

    For more information, see the multiversion compatibility documentation.

    Reviewer roulette

    Changes that require review have been detected!

    Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

    Category Reviewer Maintainer
    backend @ibaum profile link current availability (UTC-6, 7 hours behind author) @engwan profile link current availability (UTC+8, 7 hours ahead of author)
    frontend @robyrne profile link current availability (UTC+0, 1 hour behind author) @ntepluhina profile link current availability (UTC+1, same timezone as author)

    Please check reviewer's status!

    • available Reviewer is available!
    • unavailable Reviewer is unavailable!

    Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

    To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

    Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

    If needed, you can retry the :repeat: danger-review job that generated this comment.

    Generated by :no_entry_sign: Danger

  • Adam Hegyi added 188 commits

    added 188 commits

    Compare with previous version

  • Adam Hegyi changed title from WIP to Increase the page size for the contributions GraphQL query

    changed title from WIP to Increase the page size for the contributions GraphQL query

  • Adam Hegyi changed the description

    changed the description

  • 🤖 GitLab Bot 🤖 resolved all threads

    resolved all threads

  • Contributor

    Bundle size analysis [beta]

    This compares changes in bundle size for entry points between the commits f7f4979f and 44b589b8

    :sparkles: Special assets

    Entrypoint / Name Size before Size after Diff Diff in percent
    average 4.08 MB 4.08 MB - 0.0 %
    mainChunk 3.08 MB 3.08 MB - 0.0 %

    Note: We do not have exact data for f7f4979f. So we have used data from: 094a3c9f.
    The intended commit has no webpack pipeline, so we chose the last commit with one before it.

    Please look at the full report for more details


    Read more about how this report works.

    Generated by :no_entry_sign: Danger

  • Adam Hegyi requested review from @apennells

    requested review from @apennells

  • Contributor

    Allure report

    allure-report-publisher generated test report!

    e2e-test-on-gdk: :white_check_mark: test report for 44b589b8

    expand test summary
    +-----------------------------------------------------------------------+
    |                            suites summary                             |
    +------------------+--------+--------+---------+-------+-------+--------+
    |                  | passed | failed | skipped | flaky | total | result |
    +------------------+--------+--------+---------+-------+-------+--------+
    | Create           | 40     | 0      | 7       | 0     | 47    | ✅     |
    | Verify           | 32     | 0      | 0       | 0     | 32    | ✅     |
    | Govern           | 48     | 0      | 0       | 0     | 48    | ✅     |
    | Data Stores      | 22     | 0      | 0       | 0     | 22    | ✅     |
    | Plan             | 55     | 0      | 0       | 0     | 55    | ✅     |
    | Monitor          | 4      | 0      | 0       | 0     | 4     | ✅     |
    | Framework sanity | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Package          | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Manage           | 0      | 0      | 1       | 0     | 1     | ➖     |
    +------------------+--------+--------+---------+-------+-------+--------+
    | Total            | 201    | 0      | 10      | 0     | 211   | ✅     |
    +------------------+--------+--------+---------+-------+-------+--------+

    e2e-package-and-test: :white_check_mark: test report for 44b589b8

    expand test summary
    +------------------------------------------------------------+
    |                       suites summary                       |
    +-------+--------+--------+---------+-------+-------+--------+
    |       | passed | failed | skipped | flaky | total | result |
    +-------+--------+--------+---------+-------+-------+--------+
    | Plan  | 158    | 0      | 6       | 0     | 164   | ✅     |
    +-------+--------+--------+---------+-------+-------+--------+
    | Total | 158    | 0      | 6       | 0     | 164   | ✅     |
    +-------+--------+--------+---------+-------+-------+--------+

    e2e-review-qa: :white_check_mark: test report for 44b589b8

    expand test summary
    +-----------------------------------------------------------------------+
    |                            suites summary                             |
    +------------------+--------+--------+---------+-------+-------+--------+
    |                  | passed | failed | skipped | flaky | total | result |
    +------------------+--------+--------+---------+-------+-------+--------+
    | Plan             | 54     | 0      | 1       | 0     | 55    | ✅     |
    | Data Stores      | 2      | 0      | 0       | 0     | 2     | ✅     |
    | Create           | 8      | 0      | 2       | 0     | 10    | ✅     |
    | Monitor          | 4      | 0      | 0       | 0     | 4     | ✅     |
    | Package          | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Framework sanity | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Govern           | 3      | 0      | 0       | 0     | 3     | ✅     |
    +------------------+--------+--------+---------+-------+-------+--------+
    | Total            | 71     | 0      | 5       | 0     | 76    | ✅     |
    +------------------+--------+--------+---------+-------+-------+--------+
  • Adam Hegyi added 1 commit

    added 1 commit

    • abc14b70 - Allow larger page size for contributions query

    Compare with previous version

  • Adam Hegyi added 1 commit

    added 1 commit

    • fd095dbe - Allow larger page size for contributions query

    Compare with previous version

  • Adam Hegyi added 1 commit

    added 1 commit

    • 80bf927a - Allow larger page size for contributions query

    Compare with previous version

  • Adam Hegyi requested review from @cablett

    requested review from @cablett

  • Adam Hegyi requested review from @ekigbo

    requested review from @ekigbo

  • Ezekiel Kigbo
  • Adam Hegyi added 1 commit

    added 1 commit

    • 44b589b8 - Allow larger page size for contributions query

    Compare with previous version

  • Adam Hegyi requested review from @felipe_artur

    requested review from @felipe_artur

  • Ezekiel Kigbo approved this merge request

    approved this merge request

  • Felipe Cardozo approved this merge request

    approved this merge request

  • Adam Hegyi resolved all threads

    resolved all threads

  • Adam Hegyi enabled an automatic merge when the pipeline for cdf8b7c5 succeeds

    enabled an automatic merge when the pipeline for cdf8b7c5 succeeds

  • Author Maintainer

    I set MWPS since we got all the approvals.

  • merged

  • Adam Hegyi mentioned in commit 75f63d6e

    mentioned in commit 75f63d6e

  • Thong Kuah
    Thong Kuah @tkuah started a thread on the diff
  • 17 17 "The end date must be within #{NUMBER_OF_DAYS} days after the start date."
    18 18 # rubocop:enable Layout/LineLength
    19 19
    20 max_page_size 500
    21 default_page_size GitlabSchema.default_max_page_size # ensure backwards compability
    • This line seems to be causing specs not to run anymore locally:

      An error occurred while loading ./spec/models/environment_spec.rb.
      Failure/Error: type.fields[field_name.to_s.camelize(:lower)].description
      
      NoMethodError:
        undefined method `description' for nil:NilClass
      
                  type.fields[field_name.to_s.camelize(:lower)].description
                                                               ^^^^^^^^^^^^
      # ./lib/gitlab/graphql/copy_field_description.rb:17:in `copy_field_description'
      # ./ee/app/graphql/mutations/boards/scoped_board_mutation.rb:13:in `block in <module:ScopedBoardMutation>'
      # ./lib/gitlab/patch/prependable.rb:39:in `class_eval'
      # ./lib/gitlab/patch/prependable.rb:39:in `prepend_features'
      # ./ee/app/graphql/ee/mutations/boards/create.rb:11:in `prepend'
      # ./ee/app/graphql/ee/mutations/boards/create.rb:11:in `block in <module:Create>'
      # ./lib/gitlab/patch/prependable.rb:39:in `class_eval'
      # ./lib/gitlab/patch/prependable.rb:39:in `prepend_features'
      # ./config/initializers/0_inject_enterprise_edition_module.rb:45:in `prepend'
      # ./config/initializers/0_inject_enterprise_edition_module.rb:45:in `prepend_module'
      # ./config/initializers/0_inject_enterprise_edition_module.rb:8:in `block in prepend_mod_with'
      # ./config/initializers/0_inject_enterprise_edition_module.rb:60:in `block in each_extension_for'
      # ./config/initializers/0_inject_enterprise_edition_module.rb:53:in `each'
      # ./config/initializers/0_inject_enterprise_edition_module.rb:53:in `each_extension_for'
      # ./config/initializers/0_inject_enterprise_edition_module.rb:7:in `prepend_mod_with'
      # ./app/graphql/mutations/boards/create.rb:32:in `<main>'
      # /Users/tkuah/.rbenv/versions/3.1.4/lib/ruby/gems/3.1.0/gems/bootsnap-1.17.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
      # /Users/tkuah/.rbenv/versions/3.1.4/lib/ruby/gems/3.1.0/gems/zeitwerk-2.6.7/lib/zeitwerk/kernel.rb:30:in `require'
      # ./app/graphql/types/mutation_type.rb:34:in `<class:MutationType>'
      # ./app/graphql/types/mutation_type.rb:4:in `<module:Types>'
      # ./app/graphql/types/mutation_type.rb:3:in `<main>'
      # /Users/tkuah/.rbenv/versions/3.1.4/lib/ruby/gems/3.1.0/gems/bootsnap-1.17.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
      # /Users/tkuah/.rbenv/versions/3.1.4/lib/ruby/gems/3.1.0/gems/zeitwerk-2.6.7/lib/zeitwerk/kernel.rb:30:in `require'
      # ./app/graphql/gitlab_schema.rb:29:in `<class:GitlabSchema>'
      # ./app/graphql/gitlab_schema.rb:5:in `<main>'
      # /Users/tkuah/.rbenv/versions/3.1.4/lib/ruby/gems/3.1.0/gems/bootsnap-1.17.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
      # /Users/tkuah/.rbenv/versions/3.1.4/lib/ruby/gems/3.1.0/gems/zeitwerk-2.6.7/lib/zeitwerk/kernel.rb:30:in `require'
      # ./ee/app/graphql/resolvers/analytics/contribution_analytics/contributions_resolver.rb:23:in `<class:ContributionsResolver>'
      # ./ee/app/graphql/resolvers/analytics/contribution_analytics/contributions_resolver.rb:6:in `<module:ContributionAnalytics>'
      # ./ee/app/graphql/resolvers/analytics/contribution_analytics/contributions_resolver.rb:5:in `<module:Analytics>'
      # ./ee/app/graphql/resolvers/analytics/contribution_analytics/contributions_resolver.rb:4:in `<module:Resolvers>'
      # ./ee/app/graphql/resolvers/analytics/contribution_analytics/contributions_resolver.rb:3:in `<main>'
      
      

      /cc @ahegyi @cablett @shinya.maeda @jessieay

    • The workaround for now is to run specs with GITLAB_TEST_EAGER_LOAD=1

    • Maybe swap out GitlabSchema.default_max_page_size for 100 to avoid this load issue ?

      Opened !137239 (merged)

      Edited by Thong Kuah
    • @tkuah Thanks for the fix!

    • Please register or sign in to reply
  • Thong Kuah mentioned in commit bcb7603b

    mentioned in commit bcb7603b

  • Thong Kuah mentioned in merge request !137239 (merged)

    mentioned in merge request !137239 (merged)

  • added workflowstaging label and removed workflowcanary label

  • removed Solutions label

  • Please register or sign in to reply
    Loading