support encryption in S3 backup integration test #12264

laflechejonathan · 2025-07-21T19:06:57Z

We observe that encrypted backup restores don't work when the enable_read_cache knob is enabled. We can reproduce this by modifying the backup integration test to use encryption:

[15a6a7b1ea67069e55e44ab0c15be4ab] HTTP starting GET /testbucket/data/ctests/test_s3_backup_and_restore_encryption/properties/mutation_log_type ContentLen:0
Request Header: Accept: application/xml
Request Header: Content-Length: 0
Request Header: Host: localhost
Request Header: Range: bytes=4096-8191
[15a6a7b1ea67069e55e44ab0c15be4ab] HTTP code=416 early=false, time=0.001138925552368164 GET /testbucket/data/ctests/test_s3_backup_and_restore_encryption/properties/mutation_log_type contentLen=0 [192 out, response content len 413]

<Error><Code>InvalidRange</Code><Message>The requested range is not satisfiable</Message><Resource>/testbucket/data/ctests/test_s3_backup_and_restore_encryption/properties/mutation_log_type</Resource><RequestId>1753116046030225000</RequestId><Key>data/ctests/test_s3_backup_and_restore_encryption/properties/mutation_log_type</Key><BucketName>testbucket</BucketName></Error>

We are trying to read a 1-byte file at offset 4096. This occurs when you stack: read-ahead -> encryption -> s3. read-ahead has an erroneous line that tries to read past the limit of the file (offset is correct, but length is set to the block size, 1 MB). encryption then slices this 1MB block size into its encryption block size (4KB), it then passes that read down to S3, which throws the error.

The proposed fix is to clamp the length correctly when read-ahead cache issues a read.

Code-Reviewer Section

The general pull request guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

The PR has a description, explaining both the problem and the solution.
The description mentions which forms of testing were done and the testing seems reasonable.
Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or main if this is the youngest branch)
There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

foundationdb-ci · 2025-08-04T08:27:13Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: f066653
Duration 0:04:15
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; if [[ $FDB_VERSION =~ 7\.\3. ]]; then echo skip; else exit 1; fi; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-04T08:27:14Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: f066653
Duration 0:04:16
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; if [[ $FDB_VERSION =~ 7\.\3. ]]; then echo skip; else exit 1; fi; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-04T08:27:18Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: f066653
Duration 0:04:20
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; if [[ $FDB_VERSION =~ 7\.\3. ]]; then echo skip; else exit 1; fi; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2025-08-04T08:27:18Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: f066653
Duration 0:04:16
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; if [[ $FDB_VERSION =~ 7\.\3. ]]; then echo skip; else exit 1; fi; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-04T08:27:23Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: f066653
Duration 0:04:20
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; if [[ $FDB_VERSION =~ 7\.\3. ]]; then echo skip; else exit 1; fi; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-04T09:01:23Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: f066653
Duration 0:38:24
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-04T09:24:11Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: f066653
Duration 1:01:10
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

neethuhaneesha · 2025-08-05T22:09:04Z

@laflechejonathan
We have a PR for the same issue. Can you take a look at it. #12289 ?

neethuhaneesha · 2025-08-06T01:16:54Z

Backup integration test changes(supporting encryption) looks great!

laflechejonathan · 2025-08-06T05:52:28Z

@neethuhaneesha thanks for the review, yep the linked fix should address this issue as well. Would you like me to amend this PR so it just leaves the integration test enhancement?

Is there anything I should change so the test actually runs in CI with encryption enabled (it's not obvious to me if this script is run in tests or not, so you'll need to point me in the right direction)

neethuhaneesha · 2025-08-06T09:47:22Z

@neethuhaneesha thanks for the review, yep the linked fix should address this issue as well. Would you like me to amend this PR so it just leaves the integration test enhancement?

Is there anything I should change so the test actually runs in CI with encryption enabled (it's not obvious to me if this script is run in tests or not, so you'll need to point me in the right direction)

yes.. please amend the PR to include the changes to integration tests.
This test is part of CI runs in foundationdb-pr-clang test. Currently BLOBSTORE_ENABLE_READ_CACHE is true in CI. I will try change it to randomize and see if I can reproduce the issue.

laflechejonathan · 2025-08-07T04:00:30Z

OK, made the changes requested. I'm not 100% sure whether this is the right place to change how the test is invoked in CI.

note that this PR should now fail tests until #12289 merges.

I confirmed that the test passes when I merge in those changes.

foundationdb-ci · 2025-08-11T15:41:14Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: 5c9be44
Duration 0:25:26
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

akankshamahajan15 · 2025-08-11T15:51:39Z

fdbbackup/CMakeLists.txt

@@ -52,5 +52,5 @@ endif()
 if (NOT WIN32 AND NOT OPEN_FOR_IDE)
  enable_testing()
  add_test(NAME dir_backup_tests COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/tests/dir_backup_test.sh ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR})
-  add_test(NAME s3_backup_tests COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/tests/s3_backup_test.sh ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR})
+  add_test(NAME s3_backup_tests COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/tests/s3_backup_test.sh ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR} --encrypt)


It will always test with encrypt mode only @saintstack

foundationdb-ci · 2025-08-11T15:54:00Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: 5c9be44
Duration 0:38:15
Result: ❌ FAILED
Error: Error while executing command: ctest -j ${NPROC} --no-compress-output -T test --output-on-failure. Reason: exit status 8
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-11T15:54:40Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: 5c9be44
Duration 0:38:51
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-11T16:01:03Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: 5c9be44
Duration 0:45:14
Result: ❌ FAILED
Error: Error while executing command: ctest -j ${NPROC} --no-compress-output -T test --output-on-failure. Reason: exit status 8
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-11T16:04:35Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: 5c9be44
Duration 0:48:46
Result: ❌ FAILED
Error: Error while executing command: ctest -j ${NPROC} --no-compress-output -T test --output-on-failure. Reason: exit status 8
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-11T16:18:19Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: 5c9be44
Duration 1:02:34
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-11T16:28:30Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: 5c9be44
Duration 1:12:45
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

neethuhaneesha · 2025-08-11T19:22:08Z

fdbbackup/CMakeLists.txt

@@ -52,5 +52,5 @@ endif()
 if (NOT WIN32 AND NOT OPEN_FOR_IDE)
  enable_testing()
  add_test(NAME dir_backup_tests COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/tests/dir_backup_test.sh ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR})
-  add_test(NAME s3_backup_tests COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/tests/s3_backup_test.sh ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR})
+  add_test(NAME s3_backup_tests COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/tests/s3_backup_test.sh ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR} --encrypt)


Instead of hardcoding to test always with encryption, changing this flag inside the s3_backup_test.sh by randomizing it would be better, so that the testing covers both with and without encryption cases.

thanks, implemented that suggestion and merged in develop so test should now pass.

neethuhaneesha

Looks good to me! Thanks for making the changes.

foundationdb-ci · 2025-08-12T17:57:59Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: bb7d5a0
Duration 0:23:46
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-12T18:14:58Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: bb7d5a0
Duration 0:40:47
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-12T18:23:01Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: bb7d5a0
Duration 0:48:46
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-12T18:39:43Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: bb7d5a0
Duration 1:05:30
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-12T18:43:45Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: bb7d5a0
Duration 1:09:32
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2025-08-12T18:57:06Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: bb7d5a0
Duration 1:22:27
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-12T19:00:15Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: bb7d5a0
Duration 1:25:59
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

akankshamahajan15 · 2025-08-13T18:18:54Z

For Result of foundationdb-pr-clang on Linux RHEL 9
Failure is in
OldBinary="fdbserver-7.3.69" FaultInjectionEnabled="1" TestFile="tests/restarting/from_7.3.0_until_7.4.0/SnapTestRestart-1.toml"

akankshamahajan15 · 2025-08-13T18:20:34Z

Result of foundationdb-pr on Linux RHEL 9:

100% tests passed, 0 tests failed out of 73

<Trace>Ensemble stopped
</Trace>

foundationdb-ci · 2025-08-13T18:53:29Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: bb7d5a0
Duration 0:28:31
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-13T19:04:22Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: bb7d5a0
Duration 0:39:26
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-13T19:13:33Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: bb7d5a0
Duration 0:48:35
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-13T19:28:19Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: bb7d5a0
Duration 1:03:20
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-13T19:34:06Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: bb7d5a0
Duration 1:09:10
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-13T19:34:54Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: bb7d5a0
Duration 1:09:59
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-08-13T19:41:09Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: bb7d5a0
Duration 1:16:12
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

fix bug for small files in read-ahead cache

f066653

jzhou77 added the backup-restore label Jul 23, 2025

jzhou77 requested review from neethuhaneesha and akankshamahajan15 August 4, 2025 08:22

jzhou77 closed this Aug 4, 2025

jzhou77 reopened this Aug 4, 2025

Jonathan Lafleche added 2 commits August 5, 2025 15:24

misc cleanup

579f2cf

clang-format

b1941e4

laflechejonathan mentioned this pull request Aug 6, 2025

Backup encryption fix on S3 #12289

Merged

5 tasks

neethuhaneesha requested a review from saintstack August 6, 2025 09:47

laflechejonathan changed the title ~~fix bug for small files in read-ahead cache~~ support encryption in S3 backup integration test Aug 7, 2025

Jonathan Lafleche added 2 commits August 7, 2025 11:09

revert changes to AsyncFileReadAhead

83297d6

invoke test with encrypt flag

5c9be44

akankshamahajan15 closed this Aug 11, 2025

akankshamahajan15 reopened this Aug 11, 2025

akankshamahajan15 reviewed Aug 11, 2025

View reviewed changes

neethuhaneesha reviewed Aug 11, 2025

View reviewed changes

Jonathan Lafleche added 2 commits August 12, 2025 12:59

encrypt at random

e2b0c9a

Merge branch 'main' into jl/fix-read-ahead

bb7d5a0

neethuhaneesha approved these changes Aug 12, 2025

View reviewed changes

akankshamahajan15 closed this Aug 12, 2025

akankshamahajan15 reopened this Aug 12, 2025

akankshamahajan15 closed this Aug 13, 2025

akankshamahajan15 reopened this Aug 13, 2025

akankshamahajan15 merged commit e8d011a into apple:main Aug 13, 2025
7 checks passed

support encryption in S3 backup integration test #12264

support encryption in S3 backup integration test #12264

Uh oh!

Conversation

laflechejonathan commented Jul 21, 2025

Code-Reviewer Section

For Release-Branches

Uh oh!

foundationdb-ci commented Aug 4, 2025

Result of foundationdb-pr-clang on Linux RHEL 9

Uh oh!

foundationdb-ci commented Aug 4, 2025

Result of foundationdb-pr on Linux RHEL 9

Uh oh!

foundationdb-ci commented Aug 4, 2025

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Uh oh!

foundationdb-ci commented Aug 4, 2025

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Uh oh!

foundationdb-ci commented Aug 4, 2025

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Uh oh!

foundationdb-ci commented Aug 4, 2025

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Aug 4, 2025

Result of foundationdb-pr-macos on macOS Ventura 13.x

Uh oh!

neethuhaneesha commented Aug 5, 2025

Uh oh!

neethuhaneesha commented Aug 6, 2025

Uh oh!

laflechejonathan commented Aug 6, 2025

Uh oh!

neethuhaneesha commented Aug 6, 2025

Uh oh!

laflechejonathan commented Aug 7, 2025

Uh oh!

foundationdb-ci commented Aug 11, 2025

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Uh oh!

akankshamahajan15 Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

foundationdb-ci commented Aug 11, 2025

Result of foundationdb-pr-clang on Linux RHEL 9

Uh oh!

foundationdb-ci commented Aug 11, 2025

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Aug 11, 2025

Result of foundationdb-pr on Linux RHEL 9

Uh oh!

foundationdb-ci commented Aug 11, 2025

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Uh oh!

foundationdb-ci commented Aug 11, 2025

Result of foundationdb-pr-macos on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Aug 11, 2025

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Uh oh!

neethuhaneesha Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

laflechejonathan Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

neethuhaneesha left a comment

Choose a reason for hiding this comment

Uh oh!

foundationdb-ci commented Aug 12, 2025

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Uh oh!

foundationdb-ci commented Aug 12, 2025

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Aug 12, 2025

Result of foundationdb-pr-clang-arm on Linux CentOS 7