Skip to content

Max query values worker#10972

Merged
abnegate merged 1 commit into1.8.xfrom
max-query-values-worker
Dec 17, 2025
Merged

Max query values worker#10972
abnegate merged 1 commit into1.8.xfrom
max-query-values-worker

Conversation

@fogelito
Copy link
Copy Markdown
Contributor

@fogelito fogelito commented Dec 17, 2025

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
Copy Markdown
Contributor

coderabbitai bot commented Dec 17, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

This pull request introduces a new constant APP_DATABASE_QUERY_MAX_VALUES_WORKER with a value of 5000 in the constants configuration file. The constant is subsequently used to replace hardcoded batch size values and an existing general-purpose constant in two worker-related files. The changes differentiate worker-specific database query limits from the general application limit (500).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify the constant value of 5000 is intentional and appropriate for worker batch sizing
  • Confirm all substitutions in app/worker.php and src/Appwrite/Platform/Tasks/ScheduleBase.php are semantically correct
  • Check whether any other references to the hardcoded batch sizes (10_000, 499) or the general APP_DATABASE_QUERY_MAX_VALUES constant in worker contexts should also be updated

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is completely empty of meaningful content, containing only the repository template with unfilled placeholders. Fill in the PR description template with details about what the change does, the test plan, and any related issues.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Max query values worker' accurately describes the main change: introducing a worker-specific database query max values constant.
✨ 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 max-query-values-worker
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 052c8d4 and 86deab8.

📒 Files selected for processing (3)
  • app/init/constants.php (1 hunks)
  • app/worker.php (1 hunks)
  • src/Appwrite/Platform/Tasks/ScheduleBase.php (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-23T08:06:38.889Z 
Learnt from: abnegate
Repo: appwrite/appwrite PR: 10546
File: src/Appwrite/Platform/Workers/Migrations.php:144-148
Timestamp: 2025-10-23T08:06:38.889Z
Learning: In the Appwrite codebase, migration workers receive already-validated data from queued jobs. Query validation using Query::parseQueries() happens at the API endpoint level (with try-catch for QueryException) before jobs are queued, so workers in src/Appwrite/Platform/Workers/Migrations.php don't need redundant validation.

Applied to files:

  • app/worker.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan
🔇 Additional comments (3)
app/init/constants.php (1)

63-63: LGTM! Worker-specific query limit constant added.

The new constant appropriately differentiates worker context query limits (5000) from the general API limit (500), allowing workers to handle larger batch operations efficiently.

app/worker.php (1)

186-186: LGTM! Appropriate worker-specific limit for logs database.

Applying the worker-specific query limit (5000) to the logs database is correct, as this database operates in a worker context and benefits from the higher batch processing capability.

src/Appwrite/Platform/Tasks/ScheduleBase.php (1)

221-221: Good standardization of worker batch size.

Using the worker-specific constant improves code maintainability and aligns with the architecture that intentionally allows larger batch sizes (5000 vs 500) for better performance in worker contexts, where data is already pre-validated at the queue level.

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
Copy Markdown

github-actions bot commented Dec 17, 2025

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libpng 1.6.51-r0 CVE-2025-66293 HIGH
libpng-dev 1.6.51-r0 CVE-2025-66293 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 17, 2025

✨ Benchmark results

  • Requests per second: 1,212
  • Requests with 200 status code: 218,123
  • P99 latency: 0.160806226

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,212 1,284
200 218,123 231,112
P99 0.160806226 0.157310417

@abnegate abnegate merged commit 759792c into 1.8.x Dec 17, 2025
42 checks passed
@abnegate abnegate deleted the max-query-values-worker branch December 17, 2025 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abnegate abnegate abnegate approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fogelito @abnegate