Repository navigation
Upgrade queue to 5 and migrate worker configuration - #13814
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| $this->assertSame(1, Config::getParam('workers')['databases']['coroutines']); | ||
| $this->assertSame(8, Config::getParam('workers')['stats-usage']['coroutines']); |
There was a problem hiding this comment.
These assertions hard-code the exact coroutine values from app/config/workers.php instead of testing observable worker behavior. This violates the repository directive not to mirror source configuration in assertions; configuration shape and values belong in schema or static checks, while runtime tests should verify the resulting concurrency. This repository requirement must be satisfied before merging.
| $this->assertSame(1, Config::getParam('workers')['databases']['coroutines']); | |
| $this->assertSame(8, Config::getParam('workers')['stats-usage']['coroutines']); |
Context Used: Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/Platform/Workers/RegistrationTest.php
Line: 51-52
Comment:
**Configuration Values Mirrored**
These assertions hard-code the exact coroutine values from `app/config/workers.php` instead of testing observable worker behavior. This violates the repository directive not to mirror source configuration in assertions; configuration shape and values belong in schema or static checks, while runtime tests should verify the resulting concurrency. This repository requirement must be satisfied before merging.
```suggestion
```
**Context Used:** Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
✨ Benchmark resultsComparing
Per-scenario breakdown & investigation detailsMetrics below reflect the current branch (after). Δ P95 compares against the base.
Top API waits (after) |
Re-apply the stats-calculations/stats-events split on top of #13669's scheduler-based task and #13814's queue-5 worker configuration: the renamed Tasks\StatsCalculations keeps main's Utopia\Schedule\Scheduler body and subqueries hook, Schedule.php passes telemetry through, and workers.php uses the new coroutines key for both queues. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 participant
Upgrade Appwrite to Queue ^5.0.1 and Platform 1.0.0-rc24, with both released versions resolved in composer.lock. Only Queue and Platform change in the lockfile.
Worker configuration now uses
coroutines, including the mail/notification connection pool sizing reads. Existing_APP_WORKER_MAX_COROUTINESdeployment settings and per-worker concurrency values remain supported. Prefetch defaults to each queue's effective coroutine count; startup output shows both values. Databases retains one handler per worker, and combined workers retain independent per-queue caps.Delete the 263-line in-memory Redis connection fake. Concurrency tests now exercise actual Redis Lua transitions with isolated namespaces and dedicated receive/locked command connections. They cover prefetch above concurrency and shutdown returning unstarted work without increasing retry attempts. Remove obsolete priority parameters from the publisher fake and rename its synchronous batch method to
publishMany()for upstream #317. No productionenqueueMany()callers were found.Validation
Against the installed Queue 5.0.1 and Platform rc24 releases, PHP 8.5, Swoole, and Redis:
--ignore-platform-reqs; container/CI remains the full extension compatibility gate.Full Appwrite service end-to-end tests, deployment canary, and downstream Cloud Redis/Dragonfly/NATS staging validation are still required. No prefetch tuning, codec change, transport migration, or deployment setting change is included.
Rollout and rollback
Replace workers orderly for the initial canary; observe backlog age, completion rate, settlement errors, retained processing/reservations, retries and memory. Before rolling back to queue 4, stop queue 5 receives, drain running work, recover expired reservations with queue 5 maintenance, and verify reservation state is empty and processing work accounted for. Queue 4 can read ready payloads but cannot recover queue 5 reservations; setting prefetch to one does not remove that requirement.
Downstream Cloud follow-on: https://github.com/appwrite-labs/cloud/pull/5998. Larger-prefetch experiments remain in https://github.com/appwrite-labs/cloud/issues/5996.
Closes #13813.
Upstream: utopia-php/monorepo#316 and utopia-php/monorepo#317.