Skip to content

fix: add warning logs for 10k aggregation limit#2067

Merged
yogichipalkatti merged 3 commits into
mainfrom
aggregation-limit
Jul 10, 2026
Merged

fix: add warning logs for 10k aggregation limit#2067
yogichipalkatti merged 3 commits into
mainfrom
aggregation-limit

Conversation

@yogichipalkatti

@yogichipalkatti yogichipalkatti commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Temporarily fixes Issue #1548
Added warning logs to alert operators when OpenSearch terms aggregations
hit the 10,000 bucket limit in connector sync operations. This helps
detect potential data truncation issues in workspaces with >10k unique
document IDs, filenames, or connector file IDs.

Affected functions:

get_synced_file_ids_for_connector(): Warns when connector_file_id,
document_id, or filename aggregations reach the limit
get_synced_id_to_filename_map(): Warns when document_id aggregation
reaches the limit
This is an interim solution to surface the issue. A future enhancement
should implement composite aggregation with pagination to handle
arbitrarily large result sets without truncation.

Related: Connector sync may miss documents when workspace exceeds 10k
unique values per field

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of large connector result sets so file and name lookups are less likely to return incomplete data.
    • Added warnings when result counts reach a limit, helping surface cases where data may be truncated.

@github-actions github-actions Bot added backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) bug 🔴 Something isn't working. labels Jul 9, 2026
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR centralizes the OpenSearch terms aggregation bucket-size cap into a new module constant OPENSEARCH_TERMS_AGG_LIMIT (10,000) in src/api/connectors.py, replacing hardcoded values, and adds warning logs in two functions when aggregation results hit the cap and may be truncated.

Changes

Terms aggregation limit centralization

Layer / File(s) Summary
Shared aggregation limit constant
src/api/connectors.py
Adds module constant OPENSEARCH_TERMS_AGG_LIMIT = 10_000 with comments on truncation risk and future migration plans.
Truncation warnings in get_synced_file_ids_for_connector
src/api/connectors.py
Replaces hardcoded 10000 sizes with the constant for connector_file_id, document_id, and filename aggregations; adds warning logs when each bucket list hits the cap.
Truncation warning in get_synced_id_to_filename_map
src/api/connectors.py
Replaces hardcoded size with the constant for the by_document_id aggregation and adds a warning log when the bucket list hits the cap.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: warning logs for the 10k aggregation limit.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aggregation-limit

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.

@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels Jul 9, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/api/connectors.py`:
- Around line 126-131: The warning logs in the connector aggregation path
hardcode “10k” instead of reflecting OPENSEARCH_TERMS_AGG_LIMIT, so update the
warning messages in the relevant connector aggregation checks (including the
code around connector_file_id_buckets and the other matching warning calls) to
use the shared constant’s value in the message. Make the change consistently
across all four warnings so the log text stays accurate if
OPENSEARCH_TERMS_AGG_LIMIT changes, using the existing logger.warning calls and
related connector_type/returned_count context.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2954d494-3190-4c55-b005-01dcd16a8f06

📥 Commits

Reviewing files that changed from the base of the PR and between 4d1be69 and e59450e.

📒 Files selected for processing (1)
  • src/api/connectors.py

Comment thread src/api/connectors.py
Comment on lines +126 to +131
if len(connector_file_id_buckets) == OPENSEARCH_TERMS_AGG_LIMIT:
logger.warning(
"Connector file ID aggregation hit 10k limit - results may be truncated",
connector_type=connector_type,
returned_count=len(connector_file_ids),
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Warning messages hardcode "10k" instead of using the constant.

All four warning log strings (lines 128, 143, 155, 209) embed the literal "10k" even though the PR's purpose is to centralize the limit into OPENSEARCH_TERMS_AGG_LIMIT. If the constant is ever changed, the logs will be misleading. Use the constant value in the message instead.

🔧 Suggested fix
-                "Connector file ID aggregation hit 10k limit - results may be truncated",
+                "Connector file ID aggregation hit %(limit)d limit - results may be truncated",
+                limit=OPENSEARCH_TERMS_AGG_LIMIT,
                 connector_type=connector_type,
                 returned_count=len(connector_file_ids),

Apply the same pattern to the other three warning calls (lines 143, 155, 209).

Also applies to: 141-158, 207-212

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/connectors.py` around lines 126 - 131, The warning logs in the
connector aggregation path hardcode “10k” instead of reflecting
OPENSEARCH_TERMS_AGG_LIMIT, so update the warning messages in the relevant
connector aggregation checks (including the code around
connector_file_id_buckets and the other matching warning calls) to use the
shared constant’s value in the message. Make the change consistently across all
four warnings so the log text stays accurate if OPENSEARCH_TERMS_AGG_LIMIT
changes, using the existing logger.warning calls and related
connector_type/returned_count context.

@Wallgau Wallgau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great work!

@github-actions github-actions Bot added the lgtm label Jul 10, 2026
@yogichipalkatti
yogichipalkatti merged commit 9b536d0 into main Jul 10, 2026
25 checks passed
@github-actions
github-actions Bot deleted the aggregation-limit branch July 10, 2026 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) bug 🔴 Something isn't working. lgtm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants