Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17974 )
Change subject: [encryption] KUDU-3331 Encrypt file system ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/17974/2//COMMIT_MSG Commit Message: PS2: It'd be nice to see some benchmarks for how this performs. Perhaps we could select some benchmark tests to run with and without encryption to compare, and post the runtimes before and after? Here are some that come to mind as fairly disk-IO-intensive (or at least seem like they'd be affected by this patch, and may be exercise hot paths), though feel free to not use these and 'grep -r "Benchmark" . --include="*test*"' to pick some yourself: - LogBlockManagerTest.StartupBenchmark - DenseNodeTest.RunTest with various flags set (see the original commit for some example runs) - TestEnv.TestHolePunchBenchmark and/or TabletServerTest.TestDeleteTabletBenchmark - some test that exercises election storms (none come to mind, but maybe one exists already) http://gerrit.cloudera.org:8080/#/c/17974/2/src/kudu/tools/tool_action_pbc.cc File src/kudu/tools/tool_action_pbc.cc: http://gerrit.cloudera.org:8080/#/c/17974/2/src/kudu/tools/tool_action_pbc.cc@105 PS2, Line 105: : bool IsFileEncrypted(Env* env, const std::string& fname) { : // TODO(abukor): replace with real encryption check : return fname.length() > 9 && fname.compare(fname.length() - 9, 9, ".metadata") == 0; : } nit: maybe add a comment mentioning why this is the case? I don't quite follow it as is -- seems like only log block metadata is unencrypted? What about instance and path instance files? http://gerrit.cloudera.org:8080/#/c/17974/2/src/kudu/util/env.h File src/kudu/util/env.h: http://gerrit.cloudera.org:8080/#/c/17974/2/src/kudu/util/env.h@421 PS2, Line 421: true > Ah, interesting: I'd expect that everything is kept non-encrypted by defaul +1, I found this this surprising. In general we've been security-disabled by default. Why change that now? -- To view, visit http://gerrit.cloudera.org:8080/17974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I909d0c4af0c1fca0d14c99a6627842dbe2ed7524 Gerrit-Change-Number: 17974 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 27 Oct 2021 00:18:21 +0000 Gerrit-HasComments: Yes
