Add randomize_get_part_order option for auto-ranged GET - #636
Conversation
erikfuller
left a comment
There was a problem hiding this comment.
Tagged items for Kiro to review and address.
| AWS_ZERO_STRUCT(options); | ||
| options.message = message; | ||
| options.type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT; | ||
| options.randomize_get_part_order = true; |
There was a problem hiding this comment.
kiro@
This library gets wrapped in other libraries. Can you put some example code here what it would look like to use this feature from the Java or Python libraries?
There was a problem hiding this comment.
Neither Java nor Python CRT bindings currently expose force_dynamic_part_size or similar per-request boolean options yet, so this would be new plumbing in both. Here's what it would look like once added:
Java (aws-crt-java — S3MetaRequestOptions uses fluent setters):
S3MetaRequestOptions opts = new S3MetaRequestOptions()
.withMetaRequestType(MetaRequestType.GET_OBJECT)
.withHttpRequest(request)
.withRandomizeGetPartOrder(true)
.withResponseHandler(handler);
S3MetaRequest metaRequest = s3Client.makeMetaRequest(opts);Python (aws-crt-python — S3Client.make_request() uses kwargs):
s3_request = s3_client.make_request(
type=S3RequestType.GET_OBJECT,
request=http_request,
randomize_get_part_order=True,
on_done=on_done_callback,
)Both would require adding the field to their respective JNI/CPython bridge layers to pass through to the C aws_s3_meta_request_options struct.
3a0c6a4 to
632c769
Compare
|
Without out of order delivery of data to consumer this option might have big impact on perf - s3 client limits how much work can be in flight at any given point in time and current delivery mechanism will cache parts until they can be delivered. so what might end up happening on large objects is that data delivery to consumer is blocked on the next part, but because of the randomization next part might not be finished for a while, which might stall the pipeline or deadlock in the worst case (hit mem limit on how many parts we can buffer, but we cannot flush delivery queue to free up mem because next part is not ready). |
a7b4d30 to
ab187c9
Compare
Add a new boolean option 'randomize_get_part_order' to aws_s3_meta_request_options that causes the auto-ranged GET to fetch parts in a shuffled (Fisher-Yates) order instead of sequentially. The delivery order to the caller is unaffected since the streaming layer already reorders parts via its priority queue. This can reduce hot-spot contention on storage backends for large object downloads. Implementation details: - After total_num_parts is known (post-HEAD response), a shuffled array of part numbers is allocated and populated - Graceful fallback to sequential ordering on allocation failure or if the random source is unavailable - Uses aws_device_random_u32 for cryptographic-quality randomness - Only applies to AWS_S3_META_REQUEST_TYPE_GET_OBJECT
0efae3d to
50b6b04
Compare
erikfuller
left a comment
There was a problem hiding this comment.
Comments for agent review/response
| /* With 1MB parts and a 10MB object, we expect ~10 parts */ | ||
| ASSERT_TRUE(get_count >= 2); |
There was a problem hiding this comment.
kiro@
Why can't we know predictably what the get_count should be here?
There was a problem hiding this comment.
Fixed — now asserts exactly 10 parts (1MB part_size × 10MB object).
| (void)ctx; | ||
|
|
||
| /* Verify that downloading with randomize_get_part_order=true succeeds and | ||
| * that byte-range parts were NOT requested in sequential order. */ |
There was a problem hiding this comment.
kiro@
I don't think there's a validation in this test for non-sequential, only a test that all parts were retrieved.
There was a problem hiding this comment.
You're right. I attempted adding a non-sequential assertion but it's not reliable here — the metrics record completion order, not request issue order. With small 1MB parts on parallel connections, S3 responds fast enough that completion order often appears sequential despite requests going out shuffled.
The non-sequential request ordering is validated by:
- The unit test (
meta_request_auto_ranged_get_randomize_part_order) which directly tests the shuffle function - Debug logs (visible at DEBUG level) showing "Returning request for part X of 10" in shuffled order
This integration test validates the end-to-end path works correctly: all 10 parts fetched, no duplicates, no missing parts, download succeeds with correct data delivery.
There was a problem hiding this comment.
Correction to my earlier reply: the metrics are recorded in delivery order (after the priority queue reorders parts back to sequential for the caller), not completion order. So metrics will always show parts 1, 2, 3... regardless of fetch order — they can't be used to observe the shuffle.
The non-sequential fetch ordering is validated by:
- The unit test (
meta_request_auto_ranged_get_randomize_part_order) which directly testsaws_s3_shuffle_uint32_array - Debug logs showing "Returning request for part X of 10" in shuffled order
This integration test validates the end-to-end correctness: exactly 10 parts fetched, all present (valid permutation of 1..10), download succeeds with correct sequential data delivery to the caller.
5d85503 to
69d077e
Compare
69d077e to
e07f55b
Compare
Description
Adds a new
bool randomize_get_part_orderoption toaws_s3_meta_request_optionsthat causes the auto-ranged GET to fetch byte-range parts in a shuffled order instead of sequentially.Motivation
Enable better caching characteristics for large numbers of consumers downloading the same object. Without this option, consumers are blocked waiting for each sequential range. Randomizing the fetch order maximizes overall throughput when downloading the object by reducing contention on sequential byte ranges in caching layers.
Design
s_conservative_max_requests_in_flight). This ensures the delivery pipeline can always make sequential progress within its buffering capacity, avoiding stalls or deadlocks on large objects while still distributing fetch order across concurrent consumers.aws_device_random_u64for randomness via an internalaws_s3_shuffle_uint32_arrayhelper.AWS_S3_META_REQUEST_TYPE_GET_OBJECT.Testing
meta_request_auto_ranged_get_randomize_part_order— unit test of shuffle function (valid permutation, non-sequential)meta_request_shuffle_edge_cases— edge cases (empty, single element, two elements)test_s3_get_object_randomize_part_order— integration test downloading a 10MB object with 1MB parts, verifies all parts fetched successfullyBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.