Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17974 )

Change subject: [encryption] KUDU-3331 Encrypt file system
......................................................................


Patch Set 6:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/17974/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17974/6//COMMIT_MSG@42
PS6, Line 42: $ KUDU_ALLOW_SLOW_TESTS=1 ./bin/log_block_manager-test 
--gtest_filter="*StartupBenchmark*" 2>startup-bench.txt &&  grep "Time spent 
reopening" startup-bench.txt
            : Note: Google Test filter = *StartupBenchmark*
            : [==========] Running 2 tests from 1 test suite.
            : [----------] Global test environment set-up.
            : [----------] 2 tests from EncryptionEnabled/LogBlockManagerTest
            : [ RUN      ] 
EncryptionEnabled/LogBlockManagerTest.StartupBenchmark/0
            : [       OK ] 
EncryptionEnabled/LogBlockManagerTest.StartupBenchmark/0 (30192 ms)
            : [ RUN      ] 
EncryptionEnabled/LogBlockManagerTest.StartupBenchmark/1
            : [       OK ] 
EncryptionEnabled/LogBlockManagerTest.StartupBenchmark/1 (147586 ms)
            : [----------] 2 tests from EncryptionEnabled/LogBlockManagerTest 
(177779 ms total)
            :
            : [----------] Global test environment tear-down
            : [==========] 2 tests from 1 test suite ran. (177779 ms total)
            : [  PASSED  ] 2 tests.
            : I1103 17:40:00.268719 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 1.835s     user 0.110s     sys 0.041s
            : I1103 17:40:02.084342 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 1.816s     user 0.142s     sys 0.000s
            : I1103 17:40:04.027940 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 1.944s     user 0.127s     sys 0.051s
            : I1103 17:40:05.838649 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 1.811s     user 0.147s     sys 0.003s
            : I1103 17:40:07.780369 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 1.942s     user 0.140s     sys 0.054s
            : I1103 17:40:09.581300 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 1.801s     user 0.156s     sys 0.001s
            : I1103 17:40:11.495551 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 1.914s     user 0.142s     sys 0.057s
            : I1103 17:40:13.302351 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 1.807s     user 0.147s     sys 0.002s
            : I1103 17:40:15.189821 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 1.870s     user 0.136s     sys 0.046s
            : I1103 17:40:17.010360 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 1.820s     user 0.135s     sys 0.002s
            : I1103 17:40:31.574581 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 2.258s     user 0.118s     sys 0.001s
            : I1103 17:40:33.885696 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 2.311s     user 0.133s     sys 0.054s
            : I1103 17:40:36.116078 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 2.218s     user 0.132s     sys 0.015s
            : I1103 17:40:38.349752 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 2.234s     user 0.125s     sys 0.043s
            : I1103 17:40:40.558445 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 2.209s     user 0.130s     sys 0.001s
            : I1103 17:40:42.805191 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 2.247s     user 0.131s     sys 0.039s
            : I1103 17:40:44.995265 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 2.190s     user 0.146s     sys 0.000s
            : I1103 17:40:47.269686 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 2.274s     user 0.137s     sys 0.046s
            : I1103 17:40:49.522470 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 2.253s     user 0.134s     sys 0.000s
            : I1103 17:40:51.816761 31598 log_block_manager-test.cc:1074] Time 
spent reopening block manager: real 2.294s     user 0.128s     sys 0.044s
If you've drawn conclusions or summarizations about these numbers, it'd be nice 
to share those conclusions here. Otherwise it forces reviewers to look through 
all of these tests.

Looking at it myself, it seems that encryption is negligible wrt performance, 
at least when stressing many small blocks (and more metadata). Perhaps it's 
worth increasing the number of blocks too and seeing if the behavior holds.


http://gerrit.cloudera.org:8080/#/c/17974/6//COMMIT_MSG@77
PS6, Line 77: $ ./bin/dense_node-itest -num_tablets=1000 -num_seconds=240 
2>dense-node.txt | grep "Time spent restarting tserver"
Probably worth looping a few times to see if the behavior holds across several 
runs.


http://gerrit.cloudera.org:8080/#/c/17974/6//COMMIT_MSG@81
PS6, Line 81: $ perf stat --log-fd 3 -r 10  ./bin/tablet_server-test 
--gtest_filter=TabletServerTest.TestDeleteTabletBenchmark 3>&1 2>/dev/null 
>/dev/null
            :
            :  Performance counter stats for './bin/tablet_server-test 
--gtest_filter=TabletServerTest.TestDeleteTabletBenchmark' (10 runs):
Same here, any conclusions?


http://gerrit.cloudera.org:8080/#/c/17974/6/src/kudu/consensus/log_index.cc
File src/kudu/consensus/log_index.cc:

http://gerrit.cloudera.org:8080/#/c/17974/6/src/kudu/consensus/log_index.cc@112
PS6, Line 112:     opts.is_sensitive = true;
This is interesting to think about -- log indexes don't store any data, but 
rather store a map from index to location in a WAL file. As such, it's 
surprising to see it encrypted, since it stores no user data.

I suppose the same line of thinking goes for consensus metadata in general.

Is there a security concern with exposing consensus metadata and log indexing?


http://gerrit.cloudera.org:8080/#/c/17974/6/src/kudu/util/env.cc
File src/kudu/util/env.cc:

http://gerrit.cloudera.org:8080/#/c/17974/6/src/kudu/util/env.cc@30
PS6, Line 30:                                   bool encrypted) {
nit: should be 'is_sensitive' too? Same elsewhere


http://gerrit.cloudera.org:8080/#/c/17974/6/src/kudu/util/pb_util-test.cc
File src/kudu/util/pb_util-test.cc:

http://gerrit.cloudera.org:8080/#/c/17974/6/src/kudu/util/pb_util-test.cc@76
PS6, Line 76: bool is_sensitive = false
nit: should we introduce some Sensitivity enum for this or somesuch?



--
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: 6
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 03 Nov 2021 19:59:24 +0000
Gerrit-HasComments: Yes

Reply via email to