Skip to content

Commit b67ff79

Browse files
committed
some cleanup
1 parent 4c98d56 commit b67ff79

7 files changed

Lines changed: 399 additions & 113 deletions

File tree

include/aws/s3/private/s3_meta_request_impl.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,11 @@ struct aws_s3_meta_request {
319319
bool recv_file_delete_on_failure;
320320
/* When true, use O_DIRECT for writing received data to file */
321321
bool recv_file_direct_io;
322+
/* Counter for how many times we fell back from O_DIRECT to buffered I/O for a single part.
323+
* For unaligned last part: expected to be 1.
324+
* For unsupported platform: 1 on first fallback, then direct_io is disabled (no further increments).
325+
* The warning is only logged when this transitions from 0, to avoid log spam. */
326+
size_t recv_file_direct_io_fallback_count;
322327

323328
/* File I/O options. */
324329
struct aws_s3_file_io_options fio_opts;

source/s3_auto_ranged_get.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "aws/s3/private/s3_request_messages.h"
1010
#include "aws/s3/private/s3_util.h"
1111
#include <aws/common/string.h>
12+
#include <aws/common/system_info.h>
1213
#include <inttypes.h>
1314

1415
/* Dont use buffer pool when we know response size, and its below this number,
@@ -837,6 +838,16 @@ static void s_s3_auto_ranged_get_request_finished(
837838
/* Apply a buffer pool alignment to the calculated result. */
838839
out_request_optimal_range_size = aws_s3_buffer_pool_derive_aligned_buffer_size(
839840
meta_request->client->buffer_pool, out_request_optimal_range_size);
841+
/* For O_DIRECT download, also ensure page alignment.
842+
* Buffer pool typically aligns to chunk_size which is page-aligned, but apply
843+
* a defensive round-up here to guarantee the invariant. */
844+
if (meta_request->recv_file_direct_io) {
845+
size_t page_size = aws_system_info_page_size();
846+
if (out_request_optimal_range_size % page_size != 0) {
847+
out_request_optimal_range_size =
848+
((out_request_optimal_range_size / page_size) + 1) * page_size;
849+
}
850+
}
840851
AWS_LOGF_INFO(
841852
AWS_LS_S3_META_REQUEST,
842853
"id=%p: Override the part size to be optimal. part_size=%" PRIu64 ".",

source/s3_meta_request.c

Lines changed: 151 additions & 113 deletions
Large diffs are not rendered by default.

tests/CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ add_net_test_case(test_s3_get_object_file_path_direct_io)
8282
add_net_test_case(test_s3_get_object_file_path_direct_io_content_verify)
8383
add_net_test_case(test_s3_get_object_file_path_direct_io_unsupported_append)
8484
add_net_test_case(test_s3_get_object_file_path_direct_io_unsupported_write_to_position)
85+
add_net_test_case(test_s3_get_object_file_path_direct_io_multi_part)
86+
add_net_test_case(test_s3_get_object_file_path_direct_io_unaligned_part_size)
87+
add_net_test_case(test_s3_get_object_file_path_direct_io_unaligned_last_part)
8588
add_net_test_case(test_s3_get_object_empty_object)
8689
add_net_test_case(test_s3_get_object_multiple)
8790
add_net_test_case(test_s3_get_object_multiple_serial)

tests/s3_data_plane_tests.c

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,6 +1763,12 @@ static int s_test_s3_get_object_file_path_direct_io_content_verify(struct aws_al
17631763
/* S3 response checksum was validated */
17641764
ASSERT_TRUE(meta_request_test_results.did_validate);
17651765
ASSERT_INT_EQUALS(AWS_SCA_CRC32, meta_request_test_results.validation_algorithm);
1766+
/* 1 MiB single-part download: all aligned on Linux, fallback once on non-Linux */
1767+
#if defined(__linux__)
1768+
ASSERT_UINT_EQUALS(0, meta_request->recv_file_direct_io_fallback_count);
1769+
#else
1770+
ASSERT_UINT_EQUALS(1, meta_request->recv_file_direct_io_fallback_count);
1771+
#endif
17661772

17671773
aws_s3_meta_request_release(meta_request);
17681774
aws_s3_tester_wait_for_meta_request_shutdown(&tester);
@@ -1886,6 +1892,223 @@ static int s_test_s3_get_object_file_path_direct_io_unsupported_write_to_positio
18861892
return 0;
18871893
}
18881894

1895+
/* Multi-part download with O_DIRECT: download a 10MB object with 4MB part_size,
1896+
* exercises the per-part write loop. All parts are page-aligned in this case. */
1897+
AWS_TEST_CASE(
1898+
test_s3_get_object_file_path_direct_io_multi_part,
1899+
s_test_s3_get_object_file_path_direct_io_multi_part)
1900+
static int s_test_s3_get_object_file_path_direct_io_multi_part(struct aws_allocator *allocator, void *ctx) {
1901+
(void)ctx;
1902+
1903+
struct aws_s3_tester tester;
1904+
ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
1905+
1906+
struct aws_s3_tester_client_options client_options = {
1907+
.part_size = MB_TO_BYTES(4),
1908+
};
1909+
1910+
struct aws_s3_client *client = NULL;
1911+
ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client));
1912+
1913+
struct aws_s3_file_io_options fio_opts = {
1914+
.direct_io = true,
1915+
};
1916+
1917+
struct aws_s3_tester_meta_request_options get_options = {
1918+
.allocator = allocator,
1919+
.meta_request_type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT,
1920+
.validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_SUCCESS,
1921+
.client = client,
1922+
.fio_opts = &fio_opts,
1923+
.validate_get_response_checksum = true,
1924+
.expected_validate_checksum_alg = AWS_SCA_CRC32,
1925+
.finish_callback = s_s3_test_validate_checksum,
1926+
.get_options =
1927+
{
1928+
.object_path = g_pre_existing_object_10MB,
1929+
.file_on_disk = true,
1930+
},
1931+
};
1932+
1933+
struct aws_s3_meta_request_test_results meta_request_test_results;
1934+
aws_s3_meta_request_test_results_init(&meta_request_test_results, allocator);
1935+
ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &get_options, &meta_request_test_results));
1936+
ASSERT_INT_EQUALS((int64_t)MB_TO_BYTES(10), meta_request_test_results.received_file_size);
1937+
/* On Linux, all parts (4+4+2 MiB) are page-aligned so no fallback should occur.
1938+
* On non-Linux, the first write triggers UNSUPPORTED_OPERATION fallback exactly once. */
1939+
#if defined(__linux__)
1940+
ASSERT_UINT_EQUALS(0, meta_request_test_results.recv_file_direct_io_fallback_count);
1941+
#else
1942+
ASSERT_UINT_EQUALS(1, meta_request_test_results.recv_file_direct_io_fallback_count);
1943+
#endif
1944+
1945+
aws_s3_meta_request_test_results_clean_up(&meta_request_test_results);
1946+
aws_s3_client_release(client);
1947+
aws_s3_tester_clean_up(&tester);
1948+
return 0;
1949+
}
1950+
1951+
/* O_DIRECT requires part_size to be page-aligned. Verify init fails when it's not. */
1952+
AWS_TEST_CASE(
1953+
test_s3_get_object_file_path_direct_io_unaligned_part_size,
1954+
s_test_s3_get_object_file_path_direct_io_unaligned_part_size)
1955+
static int s_test_s3_get_object_file_path_direct_io_unaligned_part_size(struct aws_allocator *allocator, void *ctx) {
1956+
(void)ctx;
1957+
1958+
struct aws_s3_tester tester;
1959+
ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
1960+
1961+
/* part_size = 5 MB - 1 byte, not page-aligned */
1962+
struct aws_s3_tester_client_options client_options = {
1963+
.part_size = MB_TO_BYTES(5) - 1,
1964+
};
1965+
1966+
struct aws_s3_client *client = NULL;
1967+
ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client));
1968+
1969+
struct aws_s3_file_io_options fio_opts = {
1970+
.direct_io = true,
1971+
};
1972+
1973+
struct aws_s3_tester_meta_request_options get_options = {
1974+
.allocator = allocator,
1975+
.meta_request_type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT,
1976+
.validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE,
1977+
.client = client,
1978+
.fio_opts = &fio_opts,
1979+
.get_options =
1980+
{
1981+
.object_path = g_pre_existing_object_1MB,
1982+
.file_on_disk = true,
1983+
},
1984+
};
1985+
1986+
ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &get_options, NULL));
1987+
1988+
aws_s3_client_release(client);
1989+
aws_s3_tester_clean_up(&tester);
1990+
return 0;
1991+
}
1992+
1993+
/* Round-trip with byte-precise object size (10MiB - 1 byte). Multi-part download with O_DIRECT.
1994+
* Verifies the unaligned last-part fallback path: all but the last part go through O_DIRECT,
1995+
* the last part has unaligned length and falls back to buffered fwrite. Read back from disk
1996+
* and compare CRC32 to confirm all bytes are correct. */
1997+
AWS_TEST_CASE(
1998+
test_s3_get_object_file_path_direct_io_unaligned_last_part,
1999+
s_test_s3_get_object_file_path_direct_io_unaligned_last_part)
2000+
static int s_test_s3_get_object_file_path_direct_io_unaligned_last_part(struct aws_allocator *allocator, void *ctx) {
2001+
(void)ctx;
2002+
2003+
struct aws_s3_tester tester;
2004+
ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
2005+
2006+
/* 4 MB part_size: 10 MiB - 1 = 9 parts of 4 MiB + 1 last part of (10 MiB - 1 - 8 MiB) bytes.
2007+
* Wait: 10 MiB = 10485760, 4 MiB = 4194304. 10485760 - 1 = 10485759.
2008+
* 10485759 / 4194304 = 2 full parts (2 * 4194304 = 8388608), remainder = 2097151 bytes.
2009+
* Last part = 2097151 = 2 MiB - 1 byte (NOT page-aligned). */
2010+
struct aws_s3_tester_client_options client_options = {
2011+
.part_size = MB_TO_BYTES(4),
2012+
};
2013+
2014+
struct aws_s3_client *client = NULL;
2015+
ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client));
2016+
2017+
/* Upload 10 MiB - 1 byte */
2018+
size_t object_size = MB_TO_BYTES(10) - 1;
2019+
struct aws_byte_buf upload_buf;
2020+
aws_s3_create_test_buffer(allocator, object_size, &upload_buf);
2021+
struct aws_byte_cursor upload_cursor = aws_byte_cursor_from_buf(&upload_buf);
2022+
struct aws_input_stream *upload_stream = aws_input_stream_new_from_cursor(allocator, &upload_cursor);
2023+
2024+
struct aws_byte_buf path_buf;
2025+
AWS_ZERO_STRUCT(path_buf);
2026+
ASSERT_SUCCESS(aws_s3_tester_upload_file_path_init(
2027+
allocator, &path_buf, aws_byte_cursor_from_c_str("/prefix/round_trip/direct_io_unaligned_last.bin")));
2028+
struct aws_byte_cursor object_path = aws_byte_cursor_from_buf(&path_buf);
2029+
2030+
struct aws_string *host_name =
2031+
aws_s3_tester_build_endpoint_string(allocator, &g_test_bucket_name, &g_test_s3_region);
2032+
struct aws_byte_cursor host_cursor = aws_byte_cursor_from_string(host_name);
2033+
2034+
/* Use the lower-level API to upload the byte-precise body */
2035+
struct aws_http_message *put_message = aws_s3_test_put_object_request_new(
2036+
allocator, &host_cursor, object_path, g_test_body_content_type, upload_stream, 0);
2037+
2038+
struct aws_s3_meta_request_options put_options = {
2039+
.type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT,
2040+
.message = put_message,
2041+
};
2042+
2043+
struct aws_s3_meta_request_test_results put_results;
2044+
aws_s3_meta_request_test_results_init(&put_results, allocator);
2045+
ASSERT_SUCCESS(aws_s3_tester_bind_meta_request(&tester, &put_options, &put_results));
2046+
2047+
struct aws_s3_meta_request *put_request = aws_s3_client_make_meta_request(client, &put_options);
2048+
ASSERT_NOT_NULL(put_request);
2049+
aws_s3_tester_wait_for_meta_request_finish(&tester);
2050+
ASSERT_INT_EQUALS(AWS_ERROR_SUCCESS, put_results.finished_error_code);
2051+
aws_s3_meta_request_release(put_request);
2052+
aws_s3_tester_wait_for_meta_request_shutdown(&tester);
2053+
aws_http_message_release(put_message);
2054+
aws_s3_meta_request_test_results_clean_up(&put_results);
2055+
2056+
/* Download with O_DIRECT to a known path */
2057+
const char *local_file_path = "aws_s3_direct_io_unaligned_last_test_file";
2058+
struct aws_http_message *get_message = aws_s3_test_get_object_request_new(allocator, host_cursor, object_path);
2059+
2060+
struct aws_s3_file_io_options fio_opts = {
2061+
.direct_io = true,
2062+
};
2063+
struct aws_s3_meta_request_options get_options = {
2064+
.type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT,
2065+
.message = get_message,
2066+
.recv_filepath = aws_byte_cursor_from_c_str(local_file_path),
2067+
.fio_opts = &fio_opts,
2068+
};
2069+
2070+
struct aws_s3_meta_request_test_results get_results;
2071+
aws_s3_meta_request_test_results_init(&get_results, allocator);
2072+
ASSERT_SUCCESS(aws_s3_tester_bind_meta_request(&tester, &get_options, &get_results));
2073+
2074+
struct aws_s3_meta_request *get_request = aws_s3_client_make_meta_request(client, &get_options);
2075+
ASSERT_NOT_NULL(get_request);
2076+
aws_s3_tester_wait_for_meta_request_finish(&tester);
2077+
ASSERT_INT_EQUALS(AWS_ERROR_SUCCESS, get_results.finished_error_code);
2078+
/* On Linux: 2 aligned parts go through O_DIRECT, 1 unaligned last part falls back. count == 1.
2079+
* On non-Linux: first write triggers UNSUPPORTED_OPERATION fallback, count == 1. */
2080+
ASSERT_UINT_EQUALS(1, get_request->recv_file_direct_io_fallback_count);
2081+
aws_s3_meta_request_release(get_request);
2082+
aws_s3_tester_wait_for_meta_request_shutdown(&tester);
2083+
2084+
/* Read back from disk and verify CRC32 matches the upload */
2085+
uint32_t expected_crc = aws_checksums_crc32(upload_buf.buffer, (int)upload_buf.len, 0);
2086+
2087+
FILE *verify_file = aws_fopen(local_file_path, "rb");
2088+
ASSERT_NOT_NULL(verify_file);
2089+
struct aws_byte_buf file_buf;
2090+
aws_byte_buf_init(&file_buf, allocator, object_size);
2091+
file_buf.len = fread(file_buf.buffer, 1, object_size, verify_file);
2092+
fclose(verify_file);
2093+
2094+
ASSERT_UINT_EQUALS(object_size, file_buf.len);
2095+
uint32_t actual_crc = aws_checksums_crc32(file_buf.buffer, (int)file_buf.len, 0);
2096+
ASSERT_UINT_EQUALS(expected_crc, actual_crc);
2097+
2098+
/* Cleanup */
2099+
remove(local_file_path);
2100+
aws_byte_buf_clean_up(&file_buf);
2101+
aws_s3_meta_request_test_results_clean_up(&get_results);
2102+
aws_http_message_release(get_message);
2103+
aws_string_destroy(host_name);
2104+
aws_input_stream_release(upload_stream);
2105+
aws_byte_buf_clean_up(&upload_buf);
2106+
aws_byte_buf_clean_up(&path_buf);
2107+
aws_s3_client_release(client);
2108+
aws_s3_tester_clean_up(&tester);
2109+
return 0;
2110+
}
2111+
18892112
AWS_TEST_CASE(test_s3_get_object_empty_object, s_test_s3_get_object_empty_default)
18902113
static int s_test_s3_get_object_empty_default(struct aws_allocator *allocator, void *ctx) {
18912114
(void)ctx;

tests/s3_tester.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ static void s_s3_test_meta_request_finish(
194194
meta_request_test_results->finished_error_code = result->error_code;
195195
meta_request_test_results->did_validate = result->did_validate;
196196
meta_request_test_results->validation_algorithm = result->validation_algorithm;
197+
meta_request_test_results->recv_file_direct_io_fallback_count =
198+
meta_request->recv_file_direct_io_fallback_count;
197199

198200
if (meta_request_test_results->finish_callback != NULL) {
199201
meta_request_test_results->finish_callback(meta_request, result, user_data);

tests/s3_tester.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,10 @@ struct aws_s3_meta_request_test_results {
277277
bool did_validate;
278278
enum aws_s3_checksum_algorithm validation_algorithm;
279279

280+
/* Captured from meta_request->recv_file_direct_io_fallback_count via a finish callback.
281+
* Tests can check this to verify the expected number of O_DIRECT fallbacks occurred. */
282+
size_t recv_file_direct_io_fallback_count;
283+
280284
/* Record data from progress_callback() */
281285
struct {
282286
uint64_t content_length; /* Remember progress->content_length */

0 commit comments

Comments
 (0)