8000 Fix: race condition by Meldiron · Pull Request #11336 · appwrite/appwrite · GitHub
[go: up one dir, main page]

Skip to content

Fix: race condition#11336

Merged
Meldiron merged 2 commits into1.8.xfrom
fix-race-condition-builds-worker
Feb 16, 2026
Merged

Fix: race condition#11336
Meldiron merged 2 commits into1.8.xfrom
fix-race-condition-builds-worker

Conversation

@Meldiron
Copy link
Contributor

What does this PR do?

(Provide a description of what this PR does and why it's needed.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@coderabbitai
Copy link
Contributor
coderabbitai bot commented Feb 16, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

The site screenshot queuing code in the Builds worker was moved from a location before the deployment auto-activation/ready path to a location after the deployment-ready/VCS handling path. Logging and error handling around the operation were left unchanged. The observable effect is that screenshots are now queued after deployments are marked ready (and after related Git/VCS actions) instead of before activation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description only contains the template placeholder text without any actual implementation details, test plan, or related issues filled in. Fill in the description sections with concrete details about the race condition being fixed, how the change addresses it, and steps to verify the fix.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix: race condition' is related to the changeset, which moves site screenshot queuing to fix a race condition in the build worker, but lacks specificity about what is being fixed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into 1.8.x

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-race-condition-builds-worker

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
github-actions bot commented Feb 16, 2026

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libecpg 18.1-r0 CVE-2026-2004 HIGH
libecpg 18.1-r0 CVE-2026-2005 HIGH
libecpg 18.1-r0 CVE-2026-2006 HIGH
libecpg 18.1-r0 CVE-2026-2007 HIGH
libecpg-dev 18.1-r0 CVE-2026-2004 HIGH
libecpg-dev 18.1-r0 CVE-2026-2005 HIGH
libecpg-dev 18.1-r0 CVE-2026-2006 HIGH
libecpg-dev 18.1-r0 CVE-2026-2007 HIGH
libpq 18.1-r0 CVE-2026-2004 HIGH
libpq 18.1-r0 CVE-2026-2005 HIGH
libpq 18.1-r0 CVE-2026-2006 HIGH
libpq 18.1-r0 CVE-2026-2007 HIGH
libpq-dev 18.1-r0 CVE-2026-2004 HIGH
libpq-dev 18.1-r0 CVE-2026-2005 HIGH
libpq-dev 18.1-r0 CVE-2026-2006 HIGH
libpq-dev 18.1-r0 CVE-2026-2007 HIGH
postgresql18-dev 18.1-r0 CVE-2026-2004 HIGH
postgresql18-dev 18.1-r0 CVE-2026-2005 HIGH
postgresql18-dev 18.1-r0 CVE-2026-2006 HIGH
postgresql18-dev 18.1-r0 CVE-2026-2007 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

Copy link
Contributor
@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/Appwrite/Platform/Modules/Functions/Workers/Builds.php`:
- Around line 1015-1024: The screenshot-queuing block currently runs for every
site deployment even when not activated; move the if ($resource->getCollection()
=== 'sites') { ...
$queueForScreenshots->setDeploymentId($deployment->getId())->setProject($project)->trigger();
Console::log('Site screenshot queued'); } block inside the existing if
($activateBuild) condition so screenshots are only queued for activated
deployments, and remove the trailing whitespace after the if
($resource->getCollection() === 'sites') line to fix the lint error.

@github-actions
Copy link

✨ Benchmark results

  • Requests per second: 1,797
  • Requests with 200 status code: 323,441
  • P99 latency: 0.0898195

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,797 1,008
200 323,441 181,550
P99 0.0898195 0.22817038

@Meldiron Meldiron merged commit 50bc2a2 into 1.8.x Feb 16, 2026
62 of 65 checks passed
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

0