EaR - Misc fixes found using end-to-end integration testing #9806

sfc-gh-ahusain · 2023-03-27T07:22:11Z

Description

Major changes proposed includes:

RESTClient filtering of trailing /(s) characters from input URI resource path
Avoid EKP exponential backup given RESTClient supports exponential backoffs retries for all retryable errors.
Memory allocation optimizations: 3.1. BaseCipher key management using Standalone semantics in KMSConnector interface endpoints 3.2. Optimize memcpy while looking encryption-keys in EKP endpoints
Avoid delay while starting EKP, given its criticality during cluster recovery.
Update BlobCipher to handle variable size BaseCipher buffer
Improved logging

Testing

Correctness:

devRunCorrectness - 100K (running)

Setup:

External KMS server to supply encryption keys (in-house)
Create cluster with: cluster_aware & domain_aware config

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)

Description Major changes proposed includes: 1. RESTClient filtering of trailing `/`(s) characters from input URI resource path 2. Avoid EKP exponential backup given RESTClient supports exponential backoffs retries for all retryable errors. 3. Memory allocation optimizations: 3.1. BaseCipher key management using Standalone semantics in KMSConnector interface endpoints 3.2. Optimize memcpy while looking encryption-keys in EKP endpoints 4. Avoid delay while starting EKP, given its criticality during cluster recovery. 5. Update BlobCipher to handle variable size BaseCipher buffer 6. Improved logging Testing Setup: 1. External KMS server to supply encryption keys (inhouse) 2. Create cluster with: cluster_aware & domain_aware config

foundationdb-ci · 2023-03-27T07:37:46Z

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

Commit ID: 80a16e6
Duration 0:15:23
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)

fdb-windows-ci · 2023-03-27T07:42:47Z

Doxense CI Report for Windows 10

Commit ID: 80a16e6
Result: ❌ FAILED
Build Logs (available for 30 days)

foundationdb-ci · 2023-03-27T07:46:32Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: 80a16e6
Duration 0:24:12
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 · 2023-03-27T07:52:41Z

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

Commit ID: 80a16e6
Duration 0:30:17
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 · 2023-03-27T07:56:35Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 80a16e6
Duration 0:34:16
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 · 2023-03-27T07:59:57Z

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

Commit ID: 80a16e6
Duration 0:37:36
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 · 2023-03-27T08:40:16Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 80a16e6
Duration 1:17:52
Result: ❌ FAILED
Error: Error while executing command: if $fail_test; then 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)
Cluster Test Logs zip file of the test logs (available for 30 days)

fdbclient/BlobGranuleFiles.cpp

Description Testing

Description Major changes: 1. Cleanup EKP driven exponential backup files. 2. Update EKP not to use #1. Testing

foundationdb-ci · 2023-03-27T22:44:42Z

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

Commit ID: 8c3c806
Duration 0:30:21
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)

fdb-windows-ci · 2023-03-27T22:53:58Z

Doxense CI Report for Windows 10

Commit ID: 8c3c806
Result: ❌ FAILED
Build Logs (available for 30 days)

foundationdb-ci · 2023-03-27T23:26:33Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 8c3c806
Duration 1:12:17
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 · 2023-03-27T23:28:36Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: 8c3c806
Duration 1:14: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 · 2023-03-27T23:41:30Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 8c3c806
Duration 1:27:13
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 · 2023-03-28T02:09:31Z

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

Commit ID: 6e439f7
Duration 0:13: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 · 2023-03-28T02:24:43Z

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

Commit ID: 6e439f7
Duration 0:29:03
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 · 2023-03-28T02:33:32Z

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

Commit ID: 6e439f7
Duration 0:37:52
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)

fdb-windows-ci · 2023-03-28T02:34:24Z

Doxense CI Report for Windows 10

Commit ID: 6e439f7
Result: ❌ FAILED
Build Logs (available for 30 days)

foundationdb-ci · 2023-03-28T02:40:49Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 6e439f7
Duration 0:45:07
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 · 2023-03-28T02:56:03Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: 6e439f7
Duration 1:00:17
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 · 2023-03-28T03:12:42Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 6e439f7
Duration 1:17:04
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 · 2023-03-28T05:41:28Z

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

Commit ID: 6e439f7
Duration 0:15:01
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)

Description Testing

foundationdb-ci · 2023-03-29T05:47:44Z

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

Commit ID: 131d3bf
Duration 0:22:08
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 · 2023-03-29T05:58:01Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 131d3bf
Duration 0:32:26
Result: ❌ FAILED
Error: Error while executing command: docker build --label "org.foundationdb.version=${FDB_VERSION}" --label "org.foundationdb.build_date=${BUILD_DATE}" --label "org.foundationdb.commit=${COMMIT_SHA}" --progress plain --build-arg FDB_VERSION="${FDB_VERSION}" --build-arg FDB_LIBRARY_VERSIONS="${FDB_VERSION}" --build-arg FDB_WEBSITE="${FDB_WEBSITE}" --tag foundationdb/base:${FDB_VERSION}-${COMMIT_SHA} --file Dockerfile --target base .. 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)

fdb-windows-ci · 2023-03-29T06:05:40Z

Doxense CI Report for Windows 10

Commit ID: 131d3bf
Result: ❌ FAILED
Build Logs (available for 30 days)

foundationdb-ci · 2023-03-29T06:14:27Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 131d3bf
Duration 0:48:48
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 · 2023-03-29T06:16:39Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: 131d3bf
Duration 0:51:05
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)

sfc-gh-nwijetunga

lgtm

fdbclient/BlobCipher.cpp

sfc-gh-jslocum · 2023-03-29T16:56:58Z

fdbserver/ClusterController.actor.cpp

 	loop {
 		if (self->db.serverInfo->get().encryptKeyProxy.present() && !self->recruitEncryptKeyProxy.get()) {
-			choose {
+			loop choose {


why make this a loop choose {} when every clause has a break? Isn't that just the same as prior?

It is same as usual, making it similar to other singletons for better reasoning, during integration testing triaging, we were suspecting all parts of code to explain the weird issue and thought it is better to have a common structure (all other singleton, follow this pattern)

sfc-gh-jslocum · 2023-03-29T16:58:54Z

fdbclient/include/fdbclient/EncryptKeyProxyInterface.h

 	int64_t encryptDomainId;
 	uint64_t baseCipherId;
-	StringRef baseCipherKey;
+	Standalone<StringRef> baseCipherKey;


Did we run any perf testing to ensure that the increase in string copies didn't cause any regressions? Since this seems like a hot code path? It might not matter if this is required for correctness though

We need to run perf, but given the path gets invoked on KeyMiss only and we incur penalty of reaching out to external KMS and the payload here is roughly 60sh byte per tenant; expectation is it might be fine.
Will run perf and if there are issues, will revert it back to use in-built area if needed.

fdbserver/EncryptKeyProxy.actor.cpp

Description Testing

foundationdb-ci · 2023-03-29T22:22:48Z

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

Commit ID: 9f3b89b
Duration 0:06:26
Result: ❌ FAILED
Error: Error while executing command: ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ${HOME}/.ssh_key ec2-user@${MAC_EC2_HOST} /opt/homebrew/bin/bash --login -c ./build_pr_macos.sh. 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 · 2023-03-29T22:33:22Z

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

Commit ID: 9f3b89b
Duration 0:17:00
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 · 2023-03-29T23:02:17Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: 9f3b89b
Duration 0:45:54
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 · 2023-03-29T23:02:26Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 9f3b89b
Duration 0:46:05
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)

fdb-windows-ci · 2023-03-30T00:03:05Z

Doxense CI Report for Windows 10

Commit ID: 9f3b89b
Result: ❌ FAILED
Build Logs (available for 30 days)

foundationdb-ci · 2023-03-30T00:22:43Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 9f3b89b
Duration 2:06:23
Result: ❌ FAILED
Error: Error while executing command: if $fail_test; then 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)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2023-03-30T17:54:50Z

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

Commit ID: 9f3b89b
Duration 0:14:44
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 · 2023-03-30T18:26:09Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 9f3b89b
Duration 0:45: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 · 2023-03-30T18:26:28Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: 9f3b89b
Duration 0:46:19
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 · 2023-03-30T19:43:42Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 9f3b89b
Duration 2:03:34
Result: ❌ FAILED
Error: Error while executing command: if $fail_test; then 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)
Cluster Test Logs zip file of the test logs (available for 30 days)

* EaR - Misc fixes found using end-to-end integration testing Description Major changes proposed includes: 1. RESTClient filtering of trailing `/`(s) characters from input URI resource path 2. Avoid EKP exponential backup given RESTClient supports exponential backoffs retries for all retryable errors. 3. Memory allocation optimizations: 3.1. BaseCipher key management using Standalone semantics in KMSConnector interface endpoints 3.2. Optimize memcpy while looking encryption-keys in EKP endpoints 4. Avoid delay while starting EKP, given its criticality during cluster recovery. 5. Update BlobCipher to handle variable size BaseCipher buffer 6. Improved logging Testing Setup: 1. External KMS server to supply encryption keys (inhouse) 2. Create cluster with: cluster_aware & domain_aware config * Fix EncryptionOps test Description Testing * EaR - Misc fixes found using end-to-end integration testing Description Major changes: 1. Cleanup EKP driven exponential backup files. 2. Update EKP not to use #1. Testing * EaR - Misc fixes found using end-to-end integration testing Description Address review comments Testing * Fix AES 256 key length value Description Testing * Address review comments Description Testing (cherry picked from commit 3f6fcad) (apple#9806)

…9859) * EaR - Misc fixes found using end-to-end integration testing Description Major changes proposed includes: 1. RESTClient filtering of trailing `/`(s) characters from input URI resource path 2. Avoid EKP exponential backup given RESTClient supports exponential backoffs retries for all retryable errors. 3. Memory allocation optimizations: 3.1. BaseCipher key management using Standalone semantics in KMSConnector interface endpoints 3.2. Optimize memcpy while looking encryption-keys in EKP endpoints 4. Avoid delay while starting EKP, given its criticality during cluster recovery. 5. Update BlobCipher to handle variable size BaseCipher buffer 6. Improved logging Testing Setup: 1. External KMS server to supply encryption keys (inhouse) 2. Create cluster with: cluster_aware & domain_aware config * Fix EncryptionOps test Description Testing * EaR - Misc fixes found using end-to-end integration testing Description Major changes: 1. Cleanup EKP driven exponential backup files. 2. Update EKP not to use #1. Testing * EaR - Misc fixes found using end-to-end integration testing Description Address review comments Testing * Fix AES 256 key length value Description Testing * Address review comments Description Testing (cherry picked from commit 3f6fcad) (#9806)

sfc-gh-ahusain requested review from sfc-gh-jslocum and sfc-gh-nwijetunga March 27, 2023 07:22

sfc-gh-nwijetunga reviewed Mar 27, 2023

View reviewed changes

fdbclient/BlobGranuleFiles.cpp Show resolved Hide resolved

sfc-gh-ahusain added 2 commits March 27, 2023 14:07

Fix EncryptionOps test

0eac39e

Description Testing

EaR - Misc fixes found using end-to-end integration testing

8c3c806

Description Major changes: 1. Cleanup EKP driven exponential backup files. 2. Update EKP not to use #1. Testing

Merge branch 'main' into ahusain-encryption-integration-fixes

6e439f7

sfc-gh-ahusain closed this Mar 28, 2023

sfc-gh-ahusain reopened this Mar 28, 2023

sfc-gh-ahusain requested a review from sfc-gh-nwijetunga March 28, 2023 05:26

Fix AES 256 key length value

131d3bf

Description Testing

sfc-gh-ahusain requested a review from sfc-gh-nwijetunga March 29, 2023 06:17

sfc-gh-nwijetunga previously approved these changes Mar 29, 2023

View reviewed changes

sfc-gh-jslocum reviewed Mar 29, 2023

View reviewed changes

Address review comments

9f3b89b

Description Testing

sfc-gh-ahusain dismissed sfc-gh-nwijetunga’s stale review via 9f3b89b March 29, 2023 22:16

sfc-gh-ahusain requested a review from sfc-gh-jslocum March 29, 2023 22:16

sfc-gh-ahusain closed this Mar 30, 2023

sfc-gh-ahusain reopened this Mar 30, 2023

sfc-gh-ahusain requested a review from sfc-gh-nwijetunga March 30, 2023 18:27

sfc-gh-nwijetunga approved these changes Mar 31, 2023

View reviewed changes

sfc-gh-ahusain merged commit 3f6fcad into apple:main Mar 31, 2023

sfc-gh-ahusain mentioned this pull request Mar 31, 2023

EaR - Misc fixes found using end-to-end integration testing (#9806) #9859

Merged

5 tasks

EaR - Misc fixes found using end-to-end integration testing #9806

EaR - Misc fixes found using end-to-end integration testing #9806

Uh oh!

Conversation

sfc-gh-ahusain commented Mar 27, 2023

Code-Reviewer Section

For Release-Branches

Uh oh!

foundationdb-ci commented Mar 27, 2023

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

Uh oh!

fdb-windows-ci commented Mar 27, 2023

Doxense CI Report for Windows 10

Uh oh!

foundationdb-ci commented Mar 27, 2023

Result of foundationdb-pr-clang on Linux CentOS 7

Uh oh!

foundationdb-ci commented Mar 27, 2023

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

Uh oh!

foundationdb-ci commented Mar 27, 2023

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Mar 27, 2023

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

Uh oh!

foundationdb-ci commented Mar 27, 2023

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

Uh oh!

foundationdb-ci commented Mar 27, 2023

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

Uh oh!

fdb-windows-ci commented Mar 27, 2023

Doxense CI Report for Windows 10

Uh oh!

foundationdb-ci commented Mar 27, 2023

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Mar 27, 2023

Result of foundationdb-pr-clang on Linux CentOS 7

Uh oh!

foundationdb-ci commented Mar 27, 2023

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

foundationdb-ci commented Mar 28, 2023

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

Uh oh!

foundationdb-ci commented Mar 28, 2023

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

Uh oh!

foundationdb-ci commented Mar 28, 2023

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

Uh oh!

fdb-windows-ci commented Mar 28, 2023

Doxense CI Report for Windows 10

Uh oh!

foundationdb-ci commented Mar 28, 2023

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Mar 28, 2023

Result of foundationdb-pr-clang on Linux CentOS 7

Uh oh!

foundationdb-ci commented Mar 28, 2023

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

foundationdb-ci commented Mar 28, 2023

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

Uh oh!

foundationdb-ci commented Mar 29, 2023

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

Uh oh!

foundationdb-ci commented Mar 29, 2023

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

fdb-windows-ci commented Mar 29, 2023

Doxense CI Report for Windows 10

Uh oh!

foundationdb-ci commented Mar 29, 2023

Result of foundationdb-pr on Linux CentOS 7