Skip to content

Fix project delete platform cleanup ordering#12179

Merged
ChiragAgg5k merged 2 commits into
1.9.xfrom
fix/project-delete-platform-cleanup
Apr 30, 2026
Merged

Fix project delete platform cleanup ordering#12179
ChiragAgg5k merged 2 commits into
1.9.xfrom
fix/project-delete-platform-cleanup

Conversation

@ChiragAgg5k

Copy link
Copy Markdown
Member

What does this PR do?

Moves project-level platform cleanup earlier in the deletes worker, before project database cleanup begins.

When a project is deleted, the project document is removed first and the deletes worker later cleans related resources. Previously, platform-scoped cleanup for platforms, rules, keys, webhooks, installations, repositories, vcsComments, and schedules happened after the worker opened and cleaned the project database. If project database cleanup failed before that point, those platform rows could be left behind.

This is especially visible for proxy rules because _console_rules.domain is globally unique. An orphaned rule keeps the domain reserved and causes users to see domain is already used when adding the domain to a site in another project.

This PR does not add new cleanup behavior. It moves the existing cleanup blocks earlier so they run before the project DB cleanup failure point.

Test Plan

  • Ran formatting/lint check:
composer lint src/Appwrite/Platform/Workers/Deletes.php

Result:

{"result":"pass"}
  • Started the local Appwrite stack:
docker compose up -d --force-recreate --build
  • Verified the original bug by temporarily restoring the old ordering, seeding a project and one matching _console_rules row, then invoking the real Deletes::deleteProject() path with a simulated project DB cleanup failure before the old platform cleanup section.

Old ordering result:

mode=old
rules_before=1
worker_exception=RuntimeException: simulated project DB cleanup failure
rules_after=1

This confirms the rule was orphaned when project DB cleanup failed first.

  • Restored the fixed ordering and repeated the same verification for proxy rules.

Fixed rule cleanup result:

mode=fixed
rules_before=1
Deleted 1 documents by group
worker_exception=RuntimeException: simulated project DB cleanup failure
rules_after=0

This confirms the rule is removed before the project DB cleanup failure point.

  • Broadened the verification by seeding one matching row in each moved platform collection, then invoking the real Deletes::deleteProject() path with the same simulated project DB cleanup failure.

Fixed platform cleanup result:

mode=platform-cleanup-fixed
before={"platforms":1,"rules":1,"keys":1,"webhooks":1,"installations":1,"repositories":1,"vcsComments":1,"schedules":1}
worker_exception=RuntimeException: simulated project DB cleanup failure
after={"platforms":0,"rules":0,"keys":0,"webhooks":0,"installations":0,"repositories":0,"vcsComments":0,"schedules":0}
  • Confirmed no temporary verification rows remained in MongoDB:
{
  _console_platforms: 0,
  _console_rules: 0,
  _console_keys: 0,
  _console_webhooks: 0,
  _console_installations: 0,
  _console_repositories: 0,
  _console_vcsComments: 0,
  _console_schedules: 0,
  _console_projects: 0
}

Related PRs and Issues

  • #XXXX

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?

@greptile-apps

greptile-apps Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes an orphaned-resource bug in deleteProject() by moving eight platform-collection cleanup blocks (platforms, rules, keys, webhooks, installations, repositories, vcsComments, schedules) from inside the project DB cleanup section to before it. A secondary effect is that each cleanup step — including database drops and storage directory deletes — is now wrapped in its own try/catch, so a failure in any one step is logged and execution continues rather than aborting the remainder of the cleanup.

Confidence Score: 5/5

Safe to merge — ordering fix is correct and the added error isolation is a reasonable improvement for a cleanup worker.

No P0 or P1 issues found. The reordering correctly ensures platform-scoped resources (especially the globally-unique _console_rules.domain) are cleaned before the project DB teardown can fail and leave them orphaned. The move outside the outer try block is safe because those cleanup calls only use $dbForPlatform, which is available from the start of the function. The shift from fail-fast to log-and-continue semantics is a deliberate and appropriate trade-off for a background cleanup worker.

No files require special attention.

Important Files Changed

Filename Overview
src/Appwrite/Platform/Workers/Deletes.php Moves platform-level cleanup (platforms, rules, keys, webhooks, installations, repositories, vcsComments, schedules) to before the project DB cleanup block, and wraps each step in individual try/catch blocks for resilience.

Reviews (2): Last reviewed commit: "Continue project cleanup after resource ..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Apr 29, 2026

Copy link
Copy Markdown

✨ Benchmark results

Comparing 1.9.x (before) to fix/project-delete-platform-cleanup (after).

Before

Scenario P50 (ms) P95 (ms) Requests RPS
API total 0 0 n/a n/a
Account n/a n/a n/a n/a
TablesDB n/a n/a n/a n/a
Storage n/a n/a n/a n/a
Functions n/a n/a n/a n/a

After

Scenario P50 (ms) P95 (ms) Requests RPS
API total 12.28 138.7 185 37.25
Account 22.31 162.71 35 7.7
TablesDB 11.15 18.26 35 9.35
Storage 10.44 43.02 75 19.71
Functions 17.56 28.21 40 10.55

Delta

Scenario P95 delta (ms)
API total +138.7
Account n/a
TablesDB n/a
Storage n/a
Functions n/a
Top API waits
API request Max wait (ms)
account.sessions.email.create 642.28
account.password.update 162.61
account.create 150.92

@github-actions

github-actions Bot commented Apr 29, 2026

Copy link
Copy Markdown

🔄 PHP-Retry Summary

Flaky tests detected across commits:

Commit 794d8ea - 7 flaky tests
Test Retries Total Time Details
UsageTest::testFunctionsStats 1 10.19s Logs
UsageTest::testPrepareSitesStats 1 7ms Logs
UsageTest::testEmbeddingsTextUsageDoesNotBreakProjectUsage 1 5ms Logs
FunctionsClientTest::testCreateExecution 1 3.44s Logs
FunctionsClientTest::testGetExecution 1 13ms Logs
FunctionsServerTest::testCreateExecution 1 39ms Logs
DatabaseServerTest::testBulkCreate 1 240.74s Logs
Commit 4050b9d - 4 flaky tests
Test Retries Total Time Details
UsageTest::testFunctionsStats 1 10.20s Logs
UsageTest::testPrepareSitesStats 1 7ms Logs
UsageTest::testEmbeddingsTextUsageDoesNotBreakProjectUsage 1 5ms Logs
FunctionsScheduleTest::testCreateScheduledAtExecution 1 4.03s Logs

@blacksmith-sh

This comment has been minimized.

@ChiragAgg5k ChiragAgg5k merged commit 8ffe48d into 1.9.x Apr 30, 2026
43 checks passed
@ChiragAgg5k ChiragAgg5k deleted the fix/project-delete-platform-cleanup branch April 30, 2026 04:13
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