Skip to content

Conversation

@ecrupper
Copy link
Contributor

@ecrupper ecrupper commented Sep 12, 2025

I hesitate to say "fixes" for go-vela/community#997 and go-vela/community#213 because at very very high concurrency (300 simultaneous webhooks for one repo), it's still possible to see this issue I noticed. However, these changes can handle a great deal more concurrency compared to main.

In my testing, I was able to send 30+ webhooks simultaneously and 300+ with a .25 second delay and not run into duplicate key issues.

The key difference here is removing fetches for counters and instead tying the repo counters to the create functions for hooks and builds.

This PR also forces the worker's substitution routine to play a critical role in rendering any env vars containing the build number. This breaks situations in templates when teams use those specific Vela platform vars in a sprig func like

{{ vela "BUILD_NUMBER" }}

This should really just be ${VELA_BUILD_NUMBER}, but if a user is — for whatever reason — performing some sort of inline Go logic based on build number, this would be a regression for them.

@codecov
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

❌ Patch coverage is 42.01681% with 69 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.29%. Comparing base (68f292d) to head (fa5ab7a).

Files with missing lines Patch % Lines
api/build/compile_publish.go 0.00% 23 Missing ⚠️
api/webhook/post.go 0.00% 8 Missing ⚠️
database/build/create.go 65.21% 4 Missing and 4 partials ⚠️
database/hook/create.go 65.21% 4 Missing and 4 partials ⚠️
api/build/plan.go 0.00% 5 Missing ⚠️
api/types/repo.go 60.00% 2 Missing and 2 partials ⚠️
api/repo/repair.go 0.00% 3 Missing ⚠️
scm/github/app_install.go 0.00% 3 Missing ⚠️
scm/github/repo.go 77.77% 1 Missing and 1 partial ⚠️
api/repo/create.go 0.00% 1 Missing ⚠️
... and 4 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1353      +/-   ##
==========================================
+ Coverage   58.09%   58.29%   +0.19%     
==========================================
  Files         634      633       -1     
  Lines       24577    24506      -71     
==========================================
+ Hits        14278    14285       +7     
+ Misses       9651     9565      -86     
- Partials      648      656       +8     
Files with missing lines Coverage Δ
api/hook/create.go 0.00% <ø> (ø)
api/types/build.go 98.10% <100.00%> (+<0.01%) ⬆️
database/repo/table.go 100.00% <ø> (ø)
database/types/repo.go 88.88% <100.00%> (+0.13%) ⬆️
mock/server/repo.go 0.00% <ø> (ø)
api/repo/create.go 0.00% <0.00%> (ø)
api/repo/update.go 0.00% <0.00%> (ø)
api/scm/sync.go 0.00% <0.00%> (ø)
api/scm/sync_org.go 0.00% <0.00%> (ø)
database/testutils/api_resources.go 0.00% <0.00%> (ø)
... and 9 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ecrupper ecrupper changed the title fix(database): use transactions tied to repo counters for hook and build to avoid collisions fix(database)!: use transactions tied to repo counters for hook and build to avoid collisions Sep 12, 2025
@ecrupper ecrupper marked this pull request as ready for review October 23, 2025 16:36
@ecrupper ecrupper requested a review from a team as a code owner October 23, 2025 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants