From 90eb9b866393c2a49dea6e4b6525bfd655807997 Mon Sep 17 00:00:00 2001 From: Sylvester Chin <schin@gitlab.com> Date: Tue, 4 Apr 2023 12:08:57 +0000 Subject: [PATCH] Modify cop to discourage use of `:always` data consistency - ensure good practice to try and leverage the non primary database --- .rubocop.yml | 8 +- .../worker_data_consistency.yml | 458 ++++++++++++++++++ doc/development/sidekiq/worker_attributes.md | 20 +- .../worker_data_consistency.rb | 29 +- .../worker_data_consistency_spec.rb | 123 +++-- 5 files changed, 572 insertions(+), 66 deletions(-) create mode 100644 .rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml diff --git a/.rubocop.yml b/.rubocop.yml index 18c713ea09a1c..9fcb77fb7c2b4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -979,4 +979,10 @@ Search/NamespacedClass: - 'spec/migrations/**/*.rb' - 'app/experiments/**/*_experiment.rb' - 'ee/app/experiments/**/*_experiment.rb' - - 'lib/gitlab/instrumentation/**/*.rb' \ No newline at end of file + - 'lib/gitlab/instrumentation/**/*.rb' + +SidekiqLoadBalancing/WorkerDataConsistency: + Enabled: true + Include: + - 'app/workers/**/*' + - 'ee/app/workers/**/*' diff --git a/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml b/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml new file mode 100644 index 0000000000000..e821ee190012a --- /dev/null +++ b/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml @@ -0,0 +1,458 @@ +--- +SidekiqLoadBalancing/WorkerDataConsistency: + Details: grace period + Exclude: + - 'app/workers/admin_email_worker.rb' + - 'app/workers/analytics/usage_trends/count_job_trigger_worker.rb' + - 'app/workers/analytics/usage_trends/counter_job_worker.rb' + - 'app/workers/approve_blocked_pending_approval_users_worker.rb' + - 'app/workers/authorized_keys_worker.rb' + - 'app/workers/authorized_project_update/periodic_recalculate_worker.rb' + - 'app/workers/authorized_project_update/project_recalculate_worker.rb' + - 'app/workers/authorized_project_update/user_refresh_from_replica_worker.rb' + - 'app/workers/authorized_projects_worker.rb' + - 'app/workers/auto_devops/disable_worker.rb' + - 'app/workers/auto_merge_process_worker.rb' + - 'app/workers/build_success_worker.rb' + - 'app/workers/bulk_import_worker.rb' + - 'app/workers/bulk_imports/entity_worker.rb' + - 'app/workers/bulk_imports/export_request_worker.rb' + - 'app/workers/bulk_imports/pipeline_worker.rb' + - 'app/workers/bulk_imports/relation_export_worker.rb' + - 'app/workers/bulk_imports/stuck_import_worker.rb' + - 'app/workers/chaos/cpu_spin_worker.rb' + - 'app/workers/chaos/db_spin_worker.rb' + - 'app/workers/chaos/kill_worker.rb' + - 'app/workers/chaos/leak_mem_worker.rb' + - 'app/workers/chaos/sleep_worker.rb' + - 'app/workers/chat_notification_worker.rb' + - 'app/workers/ci/archive_traces_cron_worker.rb' + - 'app/workers/ci/build_finished_worker.rb' + - 'app/workers/ci/build_prepare_worker.rb' + - 'app/workers/ci/build_schedule_worker.rb' + - 'app/workers/ci/build_trace_chunk_flush_worker.rb' + - 'app/workers/ci/cancel_pipeline_worker.rb' + - 'app/workers/ci/cancel_redundant_pipelines_worker.rb' + - 'app/workers/ci/daily_build_group_report_results_worker.rb' + - 'app/workers/ci/delete_objects_worker.rb' + - 'app/workers/ci/delete_unit_tests_worker.rb' + - 'app/workers/ci/drop_pipeline_worker.rb' + - 'app/workers/ci/external_pull_requests/create_pipeline_worker.rb' + - 'app/workers/ci/initial_pipeline_process_worker.rb' + - 'app/workers/ci/job_artifacts/expire_project_build_artifacts_worker.rb' + - 'app/workers/ci/merge_requests/add_todo_when_build_fails_worker.rb' + - 'app/workers/ci/pending_builds/update_group_worker.rb' + - 'app/workers/ci/pending_builds/update_project_worker.rb' + - 'app/workers/ci/pipeline_artifacts/coverage_report_worker.rb' + - 'app/workers/ci/pipeline_artifacts/create_quality_report_worker.rb' + - 'app/workers/ci/pipeline_artifacts/expire_artifacts_worker.rb' + - 'app/workers/ci/pipeline_success_unlock_artifacts_worker.rb' + - 'app/workers/ci/ref_delete_unlock_artifacts_worker.rb' + - 'app/workers/ci/resource_groups/assign_resource_from_resource_group_worker.rb' + - 'app/workers/ci/runners/process_runner_version_update_worker.rb' + - 'app/workers/ci/schedule_delete_objects_cron_worker.rb' + - 'app/workers/ci/stuck_builds/drop_running_worker.rb' + - 'app/workers/ci/stuck_builds/drop_scheduled_worker.rb' + - 'app/workers/ci/test_failure_history_worker.rb' + - 'app/workers/ci_platform_metrics_update_cron_worker.rb' + - 'app/workers/cleanup_container_repository_worker.rb' + - 'app/workers/cluster_configure_istio_worker.rb' + - 'app/workers/cluster_install_app_worker.rb' + - 'app/workers/cluster_patch_app_worker.rb' + - 'app/workers/cluster_provision_worker.rb' + - 'app/workers/cluster_update_app_worker.rb' + - 'app/workers/cluster_upgrade_app_worker.rb' + - 'app/workers/cluster_wait_for_app_installation_worker.rb' + - 'app/workers/cluster_wait_for_app_update_worker.rb' + - 'app/workers/cluster_wait_for_ingress_ip_address_worker.rb' + - 'app/workers/clusters/agents/delete_expired_events_worker.rb' + - 'app/workers/clusters/applications/activate_integration_worker.rb' + - 'app/workers/clusters/applications/deactivate_integration_worker.rb' + - 'app/workers/clusters/applications/uninstall_worker.rb' + - 'app/workers/clusters/applications/wait_for_uninstall_app_worker.rb' + - 'app/workers/clusters/integrations/check_prometheus_health_worker.rb' + - 'app/workers/container_expiration_policies/cleanup_container_repository_worker.rb' + - 'app/workers/container_expiration_policy_worker.rb' + - 'app/workers/container_registry/cleanup_worker.rb' + - 'app/workers/container_registry/delete_container_repository_worker.rb' + - 'app/workers/container_registry/migration/enqueuer_worker.rb' + - 'app/workers/container_registry/migration/guard_worker.rb' + - 'app/workers/counters/cleanup_refresh_worker.rb' + - 'app/workers/create_commit_signature_worker.rb' + - 'app/workers/create_note_diff_file_worker.rb' + - 'app/workers/create_pipeline_worker.rb' + - 'app/workers/database/drop_detached_partitions_worker.rb' + - 'app/workers/database/partition_management_worker.rb' + - 'app/workers/delete_container_repository_worker.rb' + - 'app/workers/delete_diff_files_worker.rb' + - 'app/workers/delete_merged_branches_worker.rb' + - 'app/workers/delete_stored_files_worker.rb' + - 'app/workers/delete_user_worker.rb' + - 'app/workers/dependency_proxy/cleanup_blob_worker.rb' + - 'app/workers/dependency_proxy/cleanup_dependency_proxy_worker.rb' + - 'app/workers/dependency_proxy/cleanup_manifest_worker.rb' + - 'app/workers/dependency_proxy/image_ttl_group_policy_worker.rb' + - 'app/workers/deployments/drop_older_deployments_worker.rb' + - 'app/workers/deployments/link_merge_request_worker.rb' + - 'app/workers/deployments/update_environment_worker.rb' + - 'app/workers/design_management/copy_design_collection_worker.rb' + - 'app/workers/design_management/new_version_worker.rb' + - 'app/workers/destroy_pages_deployments_worker.rb' + - 'app/workers/detect_repository_languages_worker.rb' + - 'app/workers/disallow_two_factor_for_group_worker.rb' + - 'app/workers/disallow_two_factor_for_subgroups_worker.rb' + - 'app/workers/email_receiver_worker.rb' + - 'app/workers/emails_on_push_worker.rb' + - 'app/workers/environments/auto_delete_cron_worker.rb' + - 'app/workers/environments/auto_stop_cron_worker.rb' + - 'app/workers/environments/auto_stop_worker.rb' + - 'app/workers/environments/canary_ingress/update_worker.rb' + - 'app/workers/error_tracking_issue_link_worker.rb' + - 'app/workers/expire_build_artifacts_worker.rb' + - 'app/workers/export_csv_worker.rb' + - 'app/workers/file_hook_worker.rb' + - 'app/workers/flush_counter_increments_worker.rb' + - 'app/workers/gitlab/export/prune_project_export_jobs_worker.rb' + - 'app/workers/gitlab/github_gists_import/finish_import_worker.rb' + - 'app/workers/gitlab/github_gists_import/import_gist_worker.rb' + - 'app/workers/gitlab/github_gists_import/start_import_worker.rb' + - 'app/workers/gitlab/github_import/advance_stage_worker.rb' + - 'app/workers/gitlab/github_import/refresh_import_jid_worker.rb' + - 'app/workers/gitlab/github_import/stage/finish_import_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_attachments_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_base_data_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_collaborators_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_issue_events_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_lfs_objects_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_notes_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_protected_branches_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_pull_requests_review_requests_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_pull_requests_reviews_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb' + - 'app/workers/gitlab/github_import/stage/import_repository_worker.rb' + - 'app/workers/gitlab/jira_import/advance_stage_worker.rb' + - 'app/workers/gitlab/jira_import/import_issue_worker.rb' + - 'app/workers/gitlab/jira_import/stage/start_import_worker.rb' + - 'app/workers/gitlab/phabricator_import/import_tasks_worker.rb' + - 'app/workers/gitlab_performance_bar_stats_worker.rb' + - 'app/workers/gitlab_service_ping_worker.rb' + - 'app/workers/gitlab_shell_worker.rb' + - 'app/workers/google_cloud/create_cloudsql_instance_worker.rb' + - 'app/workers/group_destroy_worker.rb' + - 'app/workers/group_export_worker.rb' + - 'app/workers/group_import_worker.rb' + - 'app/workers/groups/update_statistics_worker.rb' + - 'app/workers/groups/update_two_factor_requirement_for_members_worker.rb' + - 'app/workers/hashed_storage/migrator_worker.rb' + - 'app/workers/hashed_storage/project_migrate_worker.rb' + - 'app/workers/hashed_storage/project_rollback_worker.rb' + - 'app/workers/hashed_storage/rollbacker_worker.rb' + - 'app/workers/import_export_project_cleanup_worker.rb' + - 'app/workers/import_issues_csv_worker.rb' + - 'app/workers/incident_management/add_severity_system_note_worker.rb' + - 'app/workers/incident_management/close_incident_worker.rb' + - 'app/workers/incident_management/pager_duty/process_incident_worker.rb' + - 'app/workers/incident_management/process_alert_worker_v2.rb' + - 'app/workers/integrations/execute_worker.rb' + - 'app/workers/integrations/irker_worker.rb' + - 'app/workers/invalid_gpg_signature_update_worker.rb' + - 'app/workers/issuable/label_links_destroy_worker.rb' + - 'app/workers/issuable_export_csv_worker.rb' + - 'app/workers/issuables/clear_groups_issue_counter_worker.rb' + - 'app/workers/issue_due_scheduler_worker.rb' + - 'app/workers/issues/close_worker.rb' + - 'app/workers/issues/placement_worker.rb' + - 'app/workers/issues/rebalancing_worker.rb' + - 'app/workers/jira_connect/forward_event_worker.rb' + - 'app/workers/jira_connect/retry_request_worker.rb' + - 'app/workers/loose_foreign_keys/cleanup_worker.rb' + - 'app/workers/mail_scheduler/issue_due_worker.rb' + - 'app/workers/mail_scheduler/notification_service_worker.rb' + - 'app/workers/member_invitation_reminder_emails_worker.rb' + - 'app/workers/members_destroyer/unassign_issuables_worker.rb' + - 'app/workers/merge_request_cleanup_refs_worker.rb' + - 'app/workers/merge_request_mergeability_check_worker.rb' + - 'app/workers/merge_requests/close_issue_worker.rb' + - 'app/workers/merge_requests/create_pipeline_worker.rb' + - 'app/workers/merge_requests/delete_source_branch_worker.rb' + - 'app/workers/merge_requests/handle_assignees_change_worker.rb' + - 'app/workers/merge_requests/resolve_todos_worker.rb' + - 'app/workers/merge_worker.rb' + - 'app/workers/metrics/dashboard/prune_old_annotations_worker.rb' + - 'app/workers/metrics/dashboard/schedule_annotations_prune_worker.rb' + - 'app/workers/metrics/dashboard/sync_dashboards_worker.rb' + - 'app/workers/migrate_external_diffs_worker.rb' + - 'app/workers/namespaces/in_product_marketing_emails_worker.rb' + - 'app/workers/namespaces/process_sync_events_worker.rb' + - 'app/workers/namespaces/prune_aggregation_schedules_worker.rb' + - 'app/workers/namespaces/schedule_aggregation_worker.rb' + - 'app/workers/new_issue_worker.rb' + - 'app/workers/new_merge_request_worker.rb' + - 'app/workers/new_note_worker.rb' + - 'app/workers/object_pool/create_worker.rb' + - 'app/workers/object_pool/destroy_worker.rb' + - 'app/workers/object_pool/join_worker.rb' + - 'app/workers/object_pool/schedule_join_worker.rb' + - 'app/workers/object_storage/migrate_uploads_worker.rb' + - 'app/workers/onboarding/issue_created_worker.rb' + - 'app/workers/onboarding/pipeline_created_worker.rb' + - 'app/workers/onboarding/progress_worker.rb' + - 'app/workers/onboarding/user_added_worker.rb' + - 'app/workers/packages/cleanup/execute_policy_worker.rb' + - 'app/workers/packages/cleanup_package_file_worker.rb' + - 'app/workers/packages/cleanup_package_registry_worker.rb' + - 'app/workers/packages/composer/cache_cleanup_worker.rb' + - 'app/workers/packages/composer/cache_update_worker.rb' + - 'app/workers/packages/debian/cleanup_dangling_package_files_worker.rb' + - 'app/workers/packages/debian/generate_distribution_worker.rb' + - 'app/workers/packages/debian/process_changes_worker.rb' + - 'app/workers/packages/debian/process_package_file_worker.rb' + - 'app/workers/packages/go/sync_packages_worker.rb' + - 'app/workers/packages/helm/extraction_worker.rb' + - 'app/workers/packages/maven/metadata/sync_worker.rb' + - 'app/workers/packages/nuget/extraction_worker.rb' + - 'app/workers/packages/rubygems/extraction_worker.rb' + - 'app/workers/pages_domain_removal_cron_worker.rb' + - 'app/workers/pages_domain_ssl_renewal_cron_worker.rb' + - 'app/workers/pages_domain_ssl_renewal_worker.rb' + - 'app/workers/pages_domain_verification_cron_worker.rb' + - 'app/workers/pages_domain_verification_worker.rb' + - 'app/workers/pages_worker.rb' + - 'app/workers/partition_creation_worker.rb' + - 'app/workers/personal_access_tokens/expired_notification_worker.rb' + - 'app/workers/personal_access_tokens/expiring_worker.rb' + - 'app/workers/pipeline_metrics_worker.rb' + - 'app/workers/pipeline_process_worker.rb' + - 'app/workers/pipeline_schedule_worker.rb' + - 'app/workers/post_receive.rb' + - 'app/workers/process_commit_worker.rb' + - 'app/workers/project_cache_worker.rb' + - 'app/workers/project_destroy_worker.rb' + - 'app/workers/project_export_worker.rb' + - 'app/workers/projects/after_import_worker.rb' + - 'app/workers/projects/delete_branch_worker.rb' + - 'app/workers/projects/finalize_project_statistics_refresh_worker.rb' + - 'app/workers/projects/import_export/create_relation_exports_worker.rb' + - 'app/workers/projects/import_export/parallel_project_export_worker.rb' + - 'app/workers/projects/import_export/relation_export_worker.rb' + - 'app/workers/projects/import_export/wait_relation_exports_worker.rb' + - 'app/workers/projects/inactive_projects_deletion_cron_worker.rb' + - 'app/workers/projects/post_creation_worker.rb' + - 'app/workers/projects/process_sync_events_worker.rb' + - 'app/workers/projects/record_target_platforms_worker.rb' + - 'app/workers/projects/refresh_build_artifacts_size_statistics_worker.rb' + - 'app/workers/projects/schedule_bulk_repository_shard_moves_worker.rb' + - 'app/workers/projects/schedule_refresh_build_artifacts_size_statistics_worker.rb' + - 'app/workers/propagate_integration_group_worker.rb' + - 'app/workers/propagate_integration_inherit_descendant_worker.rb' + - 'app/workers/propagate_integration_inherit_worker.rb' + - 'app/workers/propagate_integration_project_worker.rb' + - 'app/workers/propagate_integration_worker.rb' + - 'app/workers/prune_old_events_worker.rb' + - 'app/workers/purge_dependency_proxy_cache_worker.rb' + - 'app/workers/rebase_worker.rb' + - 'app/workers/releases/create_evidence_worker.rb' + - 'app/workers/releases/manage_evidence_worker.rb' + - 'app/workers/remote_mirror_notification_worker.rb' + - 'app/workers/remove_expired_group_links_worker.rb' + - 'app/workers/remove_expired_members_worker.rb' + - 'app/workers/remove_unaccepted_member_invites_worker.rb' + - 'app/workers/remove_unreferenced_lfs_objects_worker.rb' + - 'app/workers/repository_archive_cache_worker.rb' + - 'app/workers/repository_check/batch_worker.rb' + - 'app/workers/repository_check/clear_worker.rb' + - 'app/workers/repository_check/dispatch_worker.rb' + - 'app/workers/repository_check/single_repository_worker.rb' + - 'app/workers/repository_cleanup_worker.rb' + - 'app/workers/repository_fork_worker.rb' + - 'app/workers/repository_import_worker.rb' + - 'app/workers/repository_update_remote_mirror_worker.rb' + - 'app/workers/run_pipeline_schedule_worker.rb' + - 'app/workers/schedule_merge_request_cleanup_refs_worker.rb' + - 'app/workers/schedule_migrate_external_diffs_worker.rb' + - 'app/workers/self_monitoring_project_create_worker.rb' + - 'app/workers/self_monitoring_project_delete_worker.rb' + - 'app/workers/service_desk_email_receiver_worker.rb' + - 'app/workers/snippets/schedule_bulk_repository_shard_moves_worker.rb' + - 'app/workers/ssh_keys/expired_notification_worker.rb' + - 'app/workers/ssh_keys/expiring_soon_notification_worker.rb' + - 'app/workers/stage_update_worker.rb' + - 'app/workers/stuck_ci_jobs_worker.rb' + - 'app/workers/stuck_export_jobs_worker.rb' + - 'app/workers/stuck_merge_jobs_worker.rb' + - 'app/workers/system_hook_push_worker.rb' + - 'app/workers/tasks_to_be_done/create_worker.rb' + - 'app/workers/terraform/states/destroy_worker.rb' + - 'app/workers/todos_destroyer/confidential_issue_worker.rb' + - 'app/workers/todos_destroyer/destroyed_designs_worker.rb' + - 'app/workers/todos_destroyer/destroyed_issuable_worker.rb' + - 'app/workers/todos_destroyer/entity_leave_worker.rb' + - 'app/workers/todos_destroyer/group_private_worker.rb' + - 'app/workers/todos_destroyer/private_features_worker.rb' + - 'app/workers/todos_destroyer/project_private_worker.rb' + - 'app/workers/trending_projects_worker.rb' + - 'app/workers/update_container_registry_info_worker.rb' + - 'app/workers/update_external_pull_requests_worker.rb' + - 'app/workers/update_head_pipeline_for_merge_request_worker.rb' + - 'app/workers/update_highest_role_worker.rb' + - 'app/workers/update_merge_requests_worker.rb' + - 'app/workers/update_project_statistics_worker.rb' + - 'app/workers/upload_checksum_worker.rb' + - 'app/workers/user_status_cleanup/batch_worker.rb' + - 'app/workers/users/create_statistics_worker.rb' + - 'app/workers/users/deactivate_dormant_users_worker.rb' + - 'app/workers/users/migrate_records_to_ghost_user_in_batches_worker.rb' + - 'app/workers/wait_for_cluster_creation_worker.rb' + - 'app/workers/web_hooks/log_destroy_worker.rb' + - 'app/workers/web_hooks/log_execution_worker.rb' + - 'app/workers/work_items/import_work_items_csv_worker.rb' + - 'app/workers/x509_certificate_revoke_worker.rb' + - 'app/workers/x509_issuer_crl_check_worker.rb' + - 'ee/app/workers/active_user_count_threshold_worker.rb' + - 'ee/app/workers/adjourned_group_deletion_worker.rb' + - 'ee/app/workers/adjourned_project_deletion_worker.rb' + - 'ee/app/workers/adjourned_projects_deletion_cron_worker.rb' + - 'ee/app/workers/admin_emails_worker.rb' + - 'ee/app/workers/analytics/code_review_metrics_worker.rb' + - 'ee/app/workers/analytics/cycle_analytics/consistency_worker.rb' + - 'ee/app/workers/analytics/cycle_analytics/incremental_worker.rb' + - 'ee/app/workers/analytics/cycle_analytics/reaggregation_worker.rb' + - 'ee/app/workers/analytics/devops_adoption/create_all_snapshots_worker.rb' + - 'ee/app/workers/analytics/devops_adoption/create_snapshot_worker.rb' + - 'ee/app/workers/app_sec/dast/profile_schedule_worker.rb' + - 'ee/app/workers/app_sec/dast/scanner_profiles_builds/consistency_worker.rb' + - 'ee/app/workers/app_sec/dast/scans/consistency_worker.rb' + - 'ee/app/workers/app_sec/dast/site_profiles_builds/consistency_worker.rb' + - 'ee/app/workers/approval_rules/external_approval_rule_payload_worker.rb' + - 'ee/app/workers/arkose/blocked_users_report_worker.rb' + - 'ee/app/workers/auth/saml_group_sync_worker.rb' + - 'ee/app/workers/automation/execute_rule_worker.rb' + - 'ee/app/workers/ci/batch_reset_minutes_worker.rb' + - 'ee/app/workers/ci/minutes/refresh_cached_data_worker.rb' + - 'ee/app/workers/ci/minutes/update_project_and_namespace_usage_worker.rb' + - 'ee/app/workers/ci/sync_reports_to_report_approval_rules_worker.rb' + - 'ee/app/workers/ci/upstream_projects_subscriptions_cleanup_worker.rb' + - 'ee/app/workers/clear_shared_runners_minutes_worker.rb' + - 'ee/app/workers/compliance_management/chain_of_custody_report_worker.rb' + - 'ee/app/workers/compliance_management/merge_requests/compliance_violations_consistency_worker.rb' + - 'ee/app/workers/compliance_management/merge_requests/compliance_violations_worker.rb' + - 'ee/app/workers/compliance_management/update_default_framework_worker.rb' + - 'ee/app/workers/create_github_webhook_worker.rb' + - 'ee/app/workers/dependencies/destroy_export_worker.rb' + - 'ee/app/workers/dependencies/export_worker.rb' + - 'ee/app/workers/deployments/auto_rollback_worker.rb' + - 'ee/app/workers/dora/daily_metrics/refresh_worker.rb' + - 'ee/app/workers/elastic/migration_worker.rb' + - 'ee/app/workers/elastic_association_indexer_worker.rb' + - 'ee/app/workers/elastic_cluster_reindexing_cron_worker.rb' + - 'ee/app/workers/elastic_commit_indexer_worker.rb' + - 'ee/app/workers/elastic_delete_project_worker.rb' + - 'ee/app/workers/elastic_full_index_worker.rb' + - 'ee/app/workers/elastic_indexing_control_worker.rb' + - 'ee/app/workers/elastic_namespace_indexer_worker.rb' + - 'ee/app/workers/elastic_namespace_rollout_worker.rb' + - 'ee/app/workers/elastic_remove_expired_namespace_subscriptions_from_index_cron_worker.rb' + - 'ee/app/workers/emails/abandoned_trial_emails_cron_worker.rb' + - 'ee/app/workers/epics/new_epic_issue_worker.rb' + - 'ee/app/workers/epics/update_cached_metadata_worker.rb' + - 'ee/app/workers/epics/update_epics_dates_worker.rb' + - 'ee/app/workers/geo/batch/project_registry_scheduler_worker.rb' + - 'ee/app/workers/geo/batch/project_registry_worker.rb' + - 'ee/app/workers/geo/batch_event_create_worker.rb' + - 'ee/app/workers/geo/container_repository_sync_worker.rb' + - 'ee/app/workers/geo/create_repository_updated_event_worker.rb' + - 'ee/app/workers/geo/design_repository_sync_worker.rb' + - 'ee/app/workers/geo/destroy_worker.rb' + - 'ee/app/workers/geo/event_worker.rb' + - 'ee/app/workers/geo/file_registry_removal_worker.rb' + - 'ee/app/workers/geo/file_removal_worker.rb' + - 'ee/app/workers/geo/hashed_storage_attachments_migration_worker.rb' + - 'ee/app/workers/geo/hashed_storage_migration_worker.rb' + - 'ee/app/workers/geo/metrics_update_worker.rb' + - 'ee/app/workers/geo/project_sync_worker.rb' + - 'ee/app/workers/geo/prune_event_log_worker.rb' + - 'ee/app/workers/geo/rename_repository_worker.rb' + - 'ee/app/workers/geo/repositories_clean_up_worker.rb' + - 'ee/app/workers/geo/repository_cleanup_worker.rb' + - 'ee/app/workers/geo/repository_verification/primary/single_worker.rb' + - 'ee/app/workers/geo/repository_verification/secondary/single_worker.rb' + - 'ee/app/workers/geo/reverification_batch_worker.rb' + - 'ee/app/workers/geo/scheduler/per_shard_scheduler_worker.rb' + - 'ee/app/workers/geo/scheduler/scheduler_worker.rb' + - 'ee/app/workers/geo/secondary/registry_consistency_worker.rb' + - 'ee/app/workers/geo/secondary_usage_data_cron_worker.rb' + - 'ee/app/workers/geo/sidekiq_cron_config_worker.rb' + - 'ee/app/workers/geo/sync_timeout_cron_worker.rb' + - 'ee/app/workers/geo/verification_batch_worker.rb' + - 'ee/app/workers/geo/verification_cron_worker.rb' + - 'ee/app/workers/geo/verification_state_backfill_worker.rb' + - 'ee/app/workers/geo/verification_timeout_worker.rb' + - 'ee/app/workers/geo/verification_worker.rb' + - 'ee/app/workers/geo_repository_destroy_worker.rb' + - 'ee/app/workers/gitlab_subscriptions/schedule_refresh_seats_worker.rb' + - 'ee/app/workers/gitlab_subscriptions/trials/apply_trial_worker.rb' + - 'ee/app/workers/group_saml_group_sync_worker.rb' + - 'ee/app/workers/groups/schedule_bulk_repository_shard_moves_worker.rb' + - 'ee/app/workers/historical_data_worker.rb' + - 'ee/app/workers/import_software_licenses_worker.rb' + - 'ee/app/workers/incident_management/apply_incident_sla_exceeded_label_worker.rb' + - 'ee/app/workers/incident_management/incident_sla_exceeded_check_worker.rb' + - 'ee/app/workers/incident_management/oncall_rotations/persist_all_rotations_shifts_job.rb' + - 'ee/app/workers/incident_management/oncall_rotations/persist_shifts_job.rb' + - 'ee/app/workers/incident_management/pending_escalations/alert_check_worker.rb' + - 'ee/app/workers/incident_management/pending_escalations/alert_create_worker.rb' + - 'ee/app/workers/incident_management/pending_escalations/issue_check_worker.rb' + - 'ee/app/workers/incident_management/pending_escalations/issue_create_worker.rb' + - 'ee/app/workers/incident_management/pending_escalations/schedule_check_cron_worker.rb' + - 'ee/app/workers/iterations/cadences/create_iterations_worker.rb' + - 'ee/app/workers/iterations/cadences/schedule_create_iterations_worker.rb' + - 'ee/app/workers/iterations/roll_over_issues_worker.rb' + - 'ee/app/workers/iterations_update_status_worker.rb' + - 'ee/app/workers/ldap_all_groups_sync_worker.rb' + - 'ee/app/workers/ldap_group_sync_worker.rb' + - 'ee/app/workers/ldap_sync_worker.rb' + - 'ee/app/workers/merge_request_reset_approvals_worker.rb' + - 'ee/app/workers/merge_requests/capture_suggested_reviewers_accepted_worker.rb' + - 'ee/app/workers/merge_requests/fetch_suggested_reviewers_worker.rb' + - 'ee/app/workers/merge_requests/sync_code_owner_approval_rules_worker.rb' + - 'ee/app/workers/merge_trains/refresh_worker.rb' + - 'ee/app/workers/namespaces/free_user_cap/backfill_notification_clearing_jobs_worker.rb' + - 'ee/app/workers/namespaces/free_user_cap/backfill_notification_jobs_worker.rb' + - 'ee/app/workers/namespaces/free_user_cap/notification_clearing_worker.rb' + - 'ee/app/workers/namespaces/free_user_cap/over_limit_notification_worker.rb' + - 'ee/app/workers/namespaces/sync_namespace_name_worker.rb' + - 'ee/app/workers/new_epic_worker.rb' + - 'ee/app/workers/onboarding/create_learn_gitlab_worker.rb' + - 'ee/app/workers/package_metadata/sync_worker.rb' + - 'ee/app/workers/personal_access_tokens/groups/policy_worker.rb' + - 'ee/app/workers/personal_access_tokens/instance/policy_worker.rb' + - 'ee/app/workers/projects/register_suggested_reviewers_project_worker.rb' + - 'ee/app/workers/refresh_license_compliance_checks_worker.rb' + - 'ee/app/workers/requirements_management/import_requirements_csv_worker.rb' + - 'ee/app/workers/requirements_management/process_requirements_reports_worker.rb' + - 'ee/app/workers/sbom/ingest_reports_worker.rb' + - 'ee/app/workers/scan_security_report_secrets_worker.rb' + - 'ee/app/workers/search/index_curation_worker.rb' + - 'ee/app/workers/security/auto_fix_worker.rb' + - 'ee/app/workers/security/create_orchestration_policy_worker.rb' + - 'ee/app/workers/security/orchestration_policy_rule_schedule_worker.rb' + - 'ee/app/workers/security/process_scan_result_policy_worker.rb' + - 'ee/app/workers/security/scans/purge_by_job_id_worker.rb' + - 'ee/app/workers/security/scans/purge_worker.rb' + - 'ee/app/workers/security/store_scans_worker.rb' + - 'ee/app/workers/security/sync_scan_policies_worker.rb' + - 'ee/app/workers/security/track_secure_scans_worker.rb' + - 'ee/app/workers/set_user_status_based_on_user_cap_setting_worker.rb' + - 'ee/app/workers/status_page/publish_worker.rb' + - 'ee/app/workers/store_security_reports_worker.rb' + - 'ee/app/workers/sync_seat_link_request_worker.rb' + - 'ee/app/workers/sync_seat_link_worker.rb' + - 'ee/app/workers/todos_destroyer/confidential_epic_worker.rb' + - 'ee/app/workers/vulnerabilities/historical_statistics/deletion_worker.rb' + - 'ee/app/workers/vulnerabilities/statistics/adjustment_worker.rb' + - 'ee/app/workers/vulnerabilities/statistics/schedule_worker.rb' + - 'ee/app/workers/vulnerability_exports/export_deletion_worker.rb' + - 'ee/app/workers/vulnerability_exports/export_worker.rb' + - 'ee/app/workers/zoekt/indexer_worker.rb' diff --git a/doc/development/sidekiq/worker_attributes.md b/doc/development/sidekiq/worker_attributes.md index a3bfe5f27cce5..1e3104c5e8671 100644 --- a/doc/development/sidekiq/worker_attributes.md +++ b/doc/development/sidekiq/worker_attributes.md @@ -242,7 +242,7 @@ can put unsustainable load on the primary database server. We therefore added th By configuring a worker's `data_consistency` field, we can then allow the scheduler to target read replicas under several strategies outlined below. -## Trading immediacy for reduced primary load +### Trading immediacy for reduced primary load We require Sidekiq workers to make an explicit decision around whether they need to use the primary database node for all reads and writes, or whether reads can be served from replicas. This is @@ -259,7 +259,8 @@ that mostly or exclusively perform writes, or workers that read their own writes into data consistency issues should a stale record be read back from a replica. **Try to avoid these scenarios, since `:always` should be considered the exception, not the rule.** -To allow for reads to be served from replicas, we added two additional consistency modes: `:sticky` and `:delayed`. +To allow for reads to be served from replicas, we added two additional consistency modes: `:sticky` and `:delayed`. A RuboCop rule +reminds the developer when `:always` data consistency mode is used. If workers require the primary database, you can disable the rule in-line. When you declare either `:sticky` or `:delayed` consistency, workers become eligible for database load-balancing. @@ -268,18 +269,17 @@ In both cases, if the replica is not up-to-date and the time from scheduling the the jobs sleep up to the minimum delay interval (0.8 seconds). This gives the replication process time to finish. The difference is in what happens when there is still replication lag after the delay: `sticky` workers switch over to the primary right away, whereas `delayed` workers fail fast and are retried once. -If they still encounter replication lag, they also switch to the primary instead. -**If your worker never performs any writes, it is strongly advised to apply one of these consistency settings, -since it never needs to rely on the primary database node.** +If the workers still encounter replication lag, they switch to the primary instead. **If your worker never performs any writes, +it is strongly advised to apply `:sticky` or `:delayed` consistency settings, since the worker never needs to rely on the primary database node.** The table below shows the `data_consistency` attribute and its values, ordered by the degree to which they prefer read replicas and wait for replicas to catch up: -| **Data Consistency** | **Description** | -|--------------|-----------------------------| -| `:always` | The job is required to use the primary database (default). It should be used for workers that primarily perform writes, have strict requirements around data consistency when reading their own writes, or are cron jobs. | -| `:sticky` | The job prefers replicas, but switches to the primary for writes or when encountering replication lag. It should be used for jobs that require to be executed as fast as possible but can sustain a small initial queuing delay. | -| `:delayed` | The job prefers replicas, but switches to the primary for writes. When encountering replication lag before the job starts, the job is retried once. If the replica is still not up to date on the next retry, it switches to the primary. It should be used for jobs where delaying execution further typically does not matter, such as cache expiration or web hooks execution. | +| **Data consistency** | **Description** | **Guideline** | +|--------------|-----------------------------|----------| +| `:always` | The job is required to use the primary database (default). | It should be used for workers that primarily perform writes, have strict requirements around data consistency when reading their own writes, or are cron jobs. | +| `:sticky` | The job prefers replicas, but switches to the primary for writes or when encountering replication lag. | It should be used for jobs that require to be executed as fast as possible but can sustain a small initial queuing delay. | +| `:delayed` | The job prefers replicas, but switches to the primary for writes. When encountering replication lag before the job starts, the job is retried once. If the replica is still not up to date on the next retry, it switches to the primary. | It should be used for jobs where delaying execution further typically does not matter, such as cache expiration or web hooks execution. | In all cases workers read either from a replica that is fully caught up, or from the primary node, so data consistency is always ensured. diff --git a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb index 7fe308d6b4021..badd81ff1381e 100644 --- a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb +++ b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb @@ -28,36 +28,29 @@ class WorkerDataConsistency < RuboCop::Cop::Base HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq/worker_attributes.html#job-data-consistency-strategies' - MSG = <<~MSG + MISSING_DATA_CONSISTENCY_MSG = <<~MSG Should define data_consistency expectation. - - It is encouraged for workers to use database replicas as much as possible by declaring - data_consistency to use the :delayed or :sticky modes. Mode :always will result in the - worker always hitting the primary database node for both reads and writes, which limits - scalability. - - Some guidelines: - - 1. If your worker mostly writes or reads its own writes, use mode :always. TRY TO AVOID THIS. - 2. If your worker performs mostly reads and can tolerate small delays, use mode :delayed. - 3. If your worker performs mostly reads but cannot tolerate any delays, use mode :sticky. - See #{HELP_LINK} for a more detailed explanation of these settings. MSG + DISCOURAGE_ALWAYS_MSG = "Refrain from using `:always` if possible." \ + "See #{HELP_LINK} for a more detailed explanation of these settings." + def_node_search :application_worker?, <<~PATTERN `(send nil? :include (const nil? :ApplicationWorker)) PATTERN - def_node_search :data_consistency_defined?, <<~PATTERN - `(send nil? :data_consistency ...) + def_node_matcher :data_consistency_value, <<~PATTERN + `(send nil? :data_consistency $(sym _) ...) PATTERN def on_class(node) - return unless in_worker?(node) && application_worker?(node) - return if data_consistency_defined?(node) + return unless application_worker?(node) + + consistency = data_consistency_value(node) + return add_offense(node, message: MISSING_DATA_CONSISTENCY_MSG) if consistency.nil? - add_offense(node) + add_offense(consistency, message: DISCOURAGE_ALWAYS_MSG) if consistency.value == :always end end end diff --git a/spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_spec.rb b/spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_spec.rb index 7b6578a074454..f41a441d6a666 100644 --- a/spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_spec.rb +++ b/spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_spec.rb @@ -3,46 +3,95 @@ require 'rubocop_spec_helper' require_relative '../../../../rubocop/cop/sidekiq_load_balancing/worker_data_consistency' -RSpec.describe RuboCop::Cop::SidekiqLoadBalancing::WorkerDataConsistency do - before do - allow(cop) - .to receive(:in_worker?) - .and_return(true) - end +RSpec.describe RuboCop::Cop::SidekiqLoadBalancing::WorkerDataConsistency, feature_category: :scalability do + context 'when data_consistency is not set' do + it 'adds an offense when not defining data_consistency' do + expect_offense(<<~CODE) + class SomeWorker + ^^^^^^^^^^^^^^^^ Should define data_consistency expectation.[...] + include ApplicationWorker - it 'adds an offense when not defining data_consistency' do - expect_offense(<<~CODE) - class SomeWorker - ^^^^^^^^^^^^^^^^ Should define data_consistency expectation.[...] - include ApplicationWorker - - queue_namespace :pipeline_hooks - feature_category :continuous_integration - urgency :high - end - CODE - end + queue_namespace :pipeline_hooks + feature_category :continuous_integration + urgency :high + end + CODE + end + + it 'adds no offense when defining data_consistency' do + expect_no_offenses(<<~CODE) + class SomeWorker + include ApplicationWorker - it 'adds no offense when defining data_consistency' do - expect_no_offenses(<<~CODE) - class SomeWorker - include ApplicationWorker - - queue_namespace :pipeline_hooks - feature_category :continuous_integration - data_consistency :delayed - urgency :high - end - CODE + queue_namespace :pipeline_hooks + feature_category :continuous_integration + data_consistency :delayed + urgency :high + end + CODE + end + + it 'adds no offense when worker is not an ApplicationWorker' do + expect_no_offenses(<<~CODE) + class SomeWorker + queue_namespace :pipeline_hooks + feature_category :continuous_integration + urgency :high + end + CODE + end end - it 'adds no offense when worker is not an ApplicationWorker' do - expect_no_offenses(<<~CODE) - class SomeWorker - queue_namespace :pipeline_hooks - feature_category :continuous_integration - urgency :high - end - CODE + context 'when data_consistency set to :always' do + it 'adds an offense when using `always` data_consistency' do + expect_offense(<<~CODE) + class SomeWorker + include ApplicationWorker + data_consistency :always + ^^^^^^^ Refrain from using `:always` if possible.[...] + + queue_namespace :pipeline_hooks + feature_category :continuous_integration + urgency :high + end + CODE + end + + it 'adds no offense when using `sticky` data_consistency' do + expect_no_offenses(<<~CODE) + class SomeWorker + include ApplicationWorker + + data_consistency :sticky + queue_namespace :pipeline_hooks + feature_category :continuous_integration + urgency :high + end + CODE + end + + it 'adds no offense when using `delayed` data_consistency' do + expect_no_offenses(<<~CODE) + class SomeWorker + include ApplicationWorker + + data_consistency :delayed + queue_namespace :pipeline_hooks + feature_category :continuous_integration + urgency :high + end + CODE + end + + it 'adds no offense when worker is not an ApplicationWorker' do + expect_no_offenses(<<~CODE) + class SomeWorker + data_consistency :always + queue_namespace :pipeline_hooks + feature_category :continuous_integration + urgency :high + end + CODE + end end end -- GitLab