Hello @andrevr 👋 This issue is slipping to 18.11. Two remaining MRs are in review now.

Write part: !225833 Read part: !225826

Hello @andrevr. I think we should add this to the 18.11 milestone unless we're already at a full capacity. I created a draft backport MR.

Hello @FIL0003 Thank you for sharing the details! Yes, that error occurred because the trigger doesn't know how to handle the conflict, as @l.rosa mentioned. However, I'm not sure why the AFTER trigger fired when the first INSERT didn't actually do anything. As far as I know, that’s usually how BEFORE triggers behave. Thanks!

Hello @hustewart Thank you! I'll be on PTO for the next two weeks, so I'll check that out when I get back. 👍

Hello @vyaklushin can you please take another look? Thanks!

Emma Park (f84ec2ff) at 13 Mar 10:30

Emma Park (9770ae52) at 13 Mar 09:45

What does this MR do and why?

Related: #592642

After the project_daily_statistics partitioning table swap in !211916, the sync trigger table_sync_function_c237afdf68() copies writes from the new partitioned table to project_daily_statistics_archived (the original table). The trigger's INSERT branch performs a plain INSERT without conflict handling.

When a row for the same (project_id, date) already exists in the archived table, the trigger raises PG::UniqueViolation, which causes an HTTP 500 on POST /git-upload-pack. This breaks all CI pipeline git clone operations after the first fetch per day per project.

This MR adds ON CONFLICT DO NOTHING to the trigger's INSERT branch, so duplicate rows in the archived table are silently skipped.

Note: This issue only affects 18.8.x. In 18.9, the archived table and trigger were removed entirely in !219031.

MR acceptance checklist

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

• This MR is backporting a bug fix, documentation update, or spec fix, previously merged in the default branch.
• The MR that fixed the bug on the default branch has been deployed to GitLab.com (not applicable for documentation or spec changes).
• The MR title is descriptive (e.g. "Backport of 'title of default branch MR'"). This is important, since the title will be copied to the patch blog post.
• Required labels have been applied to this merge request
  • severity label and bug subtype labels (if applicable)
  • If this MR fixes a bug that affects customers, the customer label has been applied.
• This MR has been approved by a maintainer (only one approval is required).
• Ensure the e2e:test-on-omnibus-ee job has succeeded, or if it has failed, investigate the failures. If you determine the failures are unrelated, you may proceed. If you need assistance investigating, reach out to a Software Engineer in Test in #s_developer_experience.

Note to the merge request author and maintainer

If you have questions about the patch release process, please:

• Refer to the patch release runbook for engineers and maintainers for guidance.
• Ask questions on the #releases Slack channel (internal only).
• Once the backport has been merged, the commit changes will be automatically deployed to a release environment that can be used for manual validation. See after merging runbook for details.

Emma Park (f413d179) at 13 Mar 09:33

... and 328 more commits

Hello @l.rosa Thanks for the ping. I think the fix works, but there's one thing I don't understand. The trigger is an after-trigger, so when the parent table receives INSERT and does nothing (because of the conflict: on_duplicate: :skip), why does the after-trigger still fire and try to INSERT into the archived table(the child table)?

Emma Park (685317b5) at 13 Mar 04:54

Thanks! 👍

Emma Park (8e5a9c81) at 13 Mar 04:37

Emma Park (10270b1e) at 13 Mar 02:39

OMG!! I swear I tested let_it_be_with_reload and it failed. That's why I changed it to let!. Now it works, whaaaat!

Emma Park (7be661e7) at 13 Mar 02:26

project.project_setting.update(push_rule: push_rule) is the existing code. I don't think database review is needed.

@vyaklushin can you please take another look? Thanks!

Some jobs keep failing but I don't think it's related to my changes. Hopefully they will be resolved before you start your review. 🤞

Thanks @vyaklushin I addressed your comments. Can you please take another look? Thanks! 🙏

Hello @mrincon thanks for reviewing. I changed the test to use project_push_rule_path(project, push_rule) and it removed your approval. Can you please re-approve it? Thanks!