Repository navigation
Allow disk use in MongoDbIO aggregations to avoid sort memory limit - #41222
mxtymoshyk wants to merge 1 commit into
Conversation
MongoDbIO now sets allowDiskUse(true) on the $bucketAuto query that computes splits and on AggregationQuery reads. Without it, servers older than 6.0 fail on any collection whose sort needs more than 100 MB, with "Sort exceeded memory limit of 104857600 bytes, but did not opt in to external sorting". Fixes apache#26495
There was a problem hiding this comment.
📋 Review Summary
This PR updates MongoDbIO (both BoundedMongoDbSource.buildAutoBuckets and AggregationQuery.apply) to explicitly enable allowDiskUse(true) on MongoDB aggregation pipelines. This prevents reads using $bucketAuto or custom aggregation pipelines on large collections from failing with the 100 MB in-memory sort limit on MongoDB servers prior to 6.0, bringing the Java connector in line with the Python and Go connectors.
🔍 General Feedback
- The implementation is clean, minimal, and directly addresses the issue in all relevant aggregation call sites.
- The unit tests using a
CommandListenerto inspect the outgoingaggregatecommand are well-designed and reliably verify the behavior without requiring a 100+ MB dataset in tests.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
Assigning reviewers: R: @kennknowles for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Fixes #26495
MongoDbIO.read().withBucketAuto(true)fails on large collections with:To split the read, the connector runs a
$bucketAutoaggregation over_id. That stage sorts the whole collection, and MongoDB limits an in-memory sort to 100 MB. Servers older than 6.0 only write the sort to disk when the request setsallowDiskUse, and the Java connector never set it. The Python connector already sets it, and the Go connector added it in #25013.This PR sets
allowDiskUse(true)in two places:BoundedMongoDbSource.buildAutoBuckets, which computes the splits. Reads that usewithQueryFn(AggregationQuery...)call it too, even withoutwithBucketAuto.AggregationQuery.apply, which runs the user's own pipeline. A large$sortor$groupthere hits the same limit.MongoDB 6.0 and later allow disk use by default (
allowDiskUseByDefault=true), so nothing changes on those servers unless an admin turned that default off. In that case Beam reads now spill to disk anyway, because the connector sets the flag on every aggregation, as the Python and Go connectors do. I can add awithAllowDiskUse(boolean)option if you'd rather let the server setting win.Atlas Free and Flex clusters ignore
allowDiskUseinstead of rejecting it (Atlas docs), so reads there behave as before. The error stays on those tiers for large collections, because they cap in-memory sorts at 32 MB and never spill to disk.Testing:
MongoDbIOTestrecord the commands the driver sends and check that theaggregatecommand carriesallowDiskUse: true. Both fail without the fix. The server hardcodes the 100 MB limit for$bucketAuto, so a small test collection can't trigger the real error../gradlew :sdks:java:io:mongodb:testpasses.