Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

EaR: Optimize logging from GetEncryptCipherKey #10326

Merged
merged 2 commits into from
May 31, 2023
Merged

EaR: Optimize logging from GetEncryptCipherKey #10326

merged 2 commits into from
May 31, 2023

Conversation

sfc-gh-ahusain
Copy link
Collaborator

Description

Optimize logging emitted from GetEncryptCipherKey module, especially the one more useful for debugging and not very useful in the production

Testing

SwizzledRollbackSideBand - randomSeed (276500218)
devRunCorrectness - 100k

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)

@sfc-gh-ahusain
EaR: Optimize logging from GetEncryptCipherKey
fe7ff49 
Description

Optimize logging emitted from GetEncryptCipherKey module,
especially the one more useful for debugging and not very useful
in the production

Testing

SwizzledRollbackSideBand - randomSeed (276500218)
devRunCorrectness - 100k
@fdb-windows-ci
Copy link
Collaborator

Doxense CI Report for Windows 10

@foundationdb-ci
Copy link
Contributor

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

  • Commit ID: fe7ff49
  • Duration 0:16:15
  • 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
Copy link
Contributor

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: fe7ff49
  • 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
Copy link
Contributor

Result of foundationdb-pr-clang on Linux CentOS 7

  • Commit ID: fe7ff49
  • Duration 0:55:31
  • 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
Copy link
Contributor

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

  • Commit ID: fe7ff49
  • Duration 1:27: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)

sfc-gh-yiwu
sfc-gh-yiwu reviewed May 24, 2023
View reviewed changes
Copy link
Collaborator

@sfc-gh-yiwu sfc-gh-yiwu left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason to comment out these loggings by default? They used to be helpful in finding out if encryption failure because of missing EKP.

@sfc-gh-ahusain
Copy link
Collaborator Author

What's the reason to comment out these loggings by default? They used to be helpful in finding out if encryption failure because of missing EKP.

Few reasons:

  1. The log line was instrumental during the feature development, however, with EKP getting more stable and reliable, the log line has not served as a great debugging assist lately.
  2. Given the log line is printed for every pending actor that attempts to fetch encryption keys (latest and/or point-lookup) and at times it causes lot of log-spew (not very useful due to Reformat the go and python bindings #1). For instance: attrition workload favoring EKP as a candidate, cold start of FDB where multiple pending actors are trying to fetch the keys simultaneously etc.

Further, we have log lines (ClusterController and EKP actor logs) tracking the EKP process lifecycle, hence, EKP missing is usually triaged using those logs in recent past, hence, suppressing these logs aren't causing any loss of debugging assistance, imo. Further, we have logs to track inserts/remove for BlobCipherKeyCaches helping constructing lifecycle of an EncryptionKey for a given run. Overall I feel logging wise we_might_ have more logging at the moment and it might be better to trim them as much as possible. This commit feels like a step in that direction.

Hope it helps.
Thanks

@foundationdb-ci
Copy link
Contributor

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

  • Commit ID: 6e09b53
  • Duration 0:27:27
  • 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
Copy link
Collaborator

Doxense CI Report for Windows 10

@foundationdb-ci
Copy link
Contributor

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: 6e09b53
  • Duration 1:29:29
  • 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
Copy link
Contributor

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

  • Commit ID: 6e09b53
  • Duration 1:32: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)
  • Cluster Test Logs zip file of the test logs (available for 30 days)

@foundationdb-ci
Copy link
Contributor

Result of foundationdb-pr-clang on Linux CentOS 7

  • Commit ID: 6e09b53
  • Duration 1:34:32
  • 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
Copy link
Contributor

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

  • Commit ID: 6e09b53
  • Duration 0:24:25
  • 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
Copy link
Contributor

Result of foundationdb-pr-clang on Linux CentOS 7

  • Commit ID: 6e09b53
  • Duration 0:56:40
  • 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
Copy link
Contributor

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: 6e09b53
  • Duration 1:18: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
Copy link
Contributor

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

  • Commit ID: 6e09b53
  • Duration 1:24:53
  • 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)

@sfc-gh-ahusain sfc-gh-ahusain merged commit 4f21e0c into apple:main May 31, 2023
sfc-gh-nwijetunga added a commit to sfc-gh-nwijetunga/foundationdb that referenced this pull request Jun 1, 2023
@sfc-gh-nwijetunga
Merge branch 'main' into nim/change-logging-level
e10bf49 
* main: (26 commits)
  Stop consistency scanner while restore is in progress
  Improve logging (apple#10387)
  Split GLOBAL_TAG_THROTTLING_FOLDING_TIME into several knobs
  Added HoltLinearSmootherImpl::eTrendFoldingTime field
  Use HoltLinearSmoother in GlobalTagThrottler
  Add HoltLinearSmoother
  EaR: Optimize logging from GetEncryptCipherKey (apple#10326)
  Fix tracing in GlobalTagThrottler::getClientRates
  Fix tracing typo in GlobalTagThrottler
  fixup! joshua_logtool error should be reported as a XML element by test_harness
  Change compiler options order to avoid -gdwarf overriding -g1 (apple#10377)
  Fix test by disable tenant
  Should repeat when speedUpSimulation is false
  Fix a simulation DR stuck issue
  update links to foundationdb.org to reference GitHub
  add cmake option to include RocksDB Tools with the Rocks DB compile
  SS Audit Storage Throttling (apple#10322)
  Fix restore range loss
  Fix type of TagCost field in BusiestReadTag traces
  Fix failing /fdbserver/TransactionTagCounter/IgnoreBelowMinRate unit test
  ...
sfc-gh-ahusain added a commit to sfc-gh-ahusain/foundationdb that referenced this pull request Jun 22, 2023
@sfc-gh-ahusain
EaR: Optimize logging from GetEncryptCipherKey (apple#10326)
9711e71 
Description

Optimize logging emitted from GetEncryptCipherKey module,
especially the one more useful for debugging and not very useful
in the production

Testing

SwizzledRollbackSideBand - randomSeed (276500218)
devRunCorrectness - 100k

(cherry picked from commit 4f21e0c)
sfc-gh-abeamon pushed a commit that referenced this pull request Jun 23, 2023
@sfc-gh-ahusain @sfc-gh-abeamon
EaR: Optimize logging from GetEncryptCipherKey (#10326)
3e611e0 
Description

Optimize logging emitted from GetEncryptCipherKey module,
especially the one more useful for debugging and not very useful
in the production

Testing

SwizzledRollbackSideBand - randomSeed (276500218)
devRunCorrectness - 100k

(cherry picked from commit 4f21e0c)
sfc-gh-jfu pushed a commit to sfc-gh-jfu/foundationdb that referenced this pull request Jul 20, 2023
@sfc-gh-ahusain
EaR: Optimize logging from GetEncryptCipherKey (apple#10326)
ca90270 
Description

Optimize logging emitted from GetEncryptCipherKey module,
especially the one more useful for debugging and not very useful
in the production

Testing

SwizzledRollbackSideBand - randomSeed (276500218)
devRunCorrectness - 100k

(cherry picked from commit 4f21e0c)
sfc-gh-jfu pushed a commit to sfc-gh-jfu/foundationdb that referenced this pull request Jul 20, 2023
@sfc-gh-ahusain
FDBCORE-5307: EaR- Optimize logging from GetEncryptCipherKey (apple#1…
a69499b 
…0326) (apple#431)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfc-gh-etschannen sfc-gh-etschannen sfc-gh-etschannen approved these changes

@sfc-gh-nwijetunga sfc-gh-nwijetunga sfc-gh-nwijetunga approved these changes

@sfc-gh-yiwu sfc-gh-yiwu sfc-gh-yiwu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sfc-gh-ahusain @fdb-windows-ci @foundationdb-ci @sfc-gh-etschannen @sfc-gh-nwijetunga @sfc-gh-yiwu