Skip to content

Guard push-ledger sweeper when appwritePushLedger is absent - #13772

Merged
ArnabChatterjee20k merged 1 commit into
mainfrom
copilot/implement-deletion-feature
Sep 19, 2026
Merged

ArnabChatterjee20k merged 1 commit into
mainfrom
copilot/implement-deletion-feature

Conversation

Copilot AI commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

deleteExpiredPushLedger could attempt deletion against projects where appwritePushLedger does not exist, relying on onError handling after entering delete flow. This change aligns the worker with the recommended guard-first pattern by short-circuiting before delete.

  • What changed

    • Added an explicit collection-existence check in Deletes::deleteExpiredPushLedger.
    • Return early when appwritePushLedger is missing, preserving existing behavior for projects where it exists.
  • Why this matters

    • Avoids delete attempts on non-existent collections.
    • Makes cleanup behavior deterministic and consistent with existing guard-before-delete patterns.
  • Code change (core)

    $dbForProject = $getProjectDB($project);
    if ($dbForProject->getCollection('appwritePushLedger')->isEmpty()) {
        return;
    }
    
    $expired = DateTime::addSeconds(new \DateTime(), -1 * 60 * 60 * 24 * 7);
    
    $dbForProject->deleteDocuments('appwritePushLedger', [
        Query::lessThan('$createdAt', $expired),
    ], onError: function (Throwable $th) {
        // Swallow errors (e.g. projects without the push ledger collection).
    });

Co-authored-by: ArnabChatterjee20k <83803257+ArnabChatterjee20k@users.noreply.github.com>
Copilot AI changed the title fix: guard push ledger sweeper when collection is missing Guard push-ledger sweeper when appwritePushLedger is absent Sep 19, 2026
@ArnabChatterjee20k
ArnabChatterjee20k marked this pull request as ready for review September 19, 2026 17:30
@greptile-apps

greptile-apps Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the new guard follows established collection-existence semantics and preserves existing cleanup behavior.

Summary

Adds an existence guard to the expired push-ledger sweeper so projects without appwritePushLedger return before attempting deletion.

  • Preserves deletion of ledger entries older than one week when the collection exists.
  • Uses the established repository pattern of checking getCollection(...)->isEmpty().
  • No security issues or repository-rule violations were identified.

Reviews (1) · Last reviewed commit: "fix: guard push ledger cleanup for missi..."

@github-actions

Copy link
Copy Markdown

✨ Benchmark results

Comparing main (before) → copilot/implement-deletion-feature (after).

Metric Before After Change
🚀 Requests/sec 272.9 268.77 -1.5%
⏱️ Latency P50 64.8 ms 65.21 ms +0.6%
⏱️ Latency P95 146.97 ms 150.72 ms +2.6%
Per-scenario breakdown & investigation details

Metrics below reflect the current branch (after). Δ P95 compares against the base.

Scenario P50 (ms) P95 (ms) Requests RPS Δ P95 (ms)
API total 65.21 150.72 16,758 268.77 +3.75
Account 121.78 226.4 882 14.7 -8.93
TablesDB 63.27 118.98 9,114 148.83 +1.52
Storage 60.09 134.67 4,410 73.43 +3.49
Functions 90.2 183 2,352 39.76 +7.39

Top API waits (after)

API request Max wait (ms)
functions.variables.update 339.84
account.name.update 335.17
functions.create 317.99
functions.delete 313.18
storage.buckets.create 298.39

@ArnabChatterjee20k
ArnabChatterjee20k merged commit a1051fd into main Sep 19, 2026
45 of 49 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.

3 participants