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

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


Patch Set 7:

(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: The following are StartupBenchmark tests run with 
KUDU_ALLOW_SLOW_TESTS
            : set to true, which uses a block count of 1,000,000.
            :
            : It seems enabling encryption adds around 20% overhead on startup 
in a
            : typical use-case with no deletes.
            :
            :  Performance counter stats for './bin/log_block_manager-test 
--gtest_filter=*StartupBenchmark/0' (10 runs):
            :
            :       40391.075316      task-clock (msec)         #    2.021 CPUs 
utilized            ( +-  1.05% )
            :             11,089      context-switches          #    0.275 
K/sec                    ( +-  9.87% )
            :                280      cpu-migrations            #    0.007 
K/sec                    ( +-  1.58% )
            :            593,982      page-faults               #    0.015 
M/sec                    ( +-  2.13% )
            :    110,595,311,391      cycles                    #    2.738 GHz  
                    ( +-  1.03% )
            :     90,580,214,722      instructions              #    0.82  insn 
per cycle           ( +-  0.14% )
            :     16,449,237,957      branches                  #  407.249 
M/sec                    ( +-  0.15% )
            :         67,169,915      branch-misses             #    0.41% of 
all branches          ( +-  0.49% )
            :
            :       19.988553457 seconds time elapsed                           
               ( +-  0.58% )
            :
            :  Performance counter stats for './bin/log_block_manager-test 
--encrypt_data_at_rest=1 --gtest_filter=*StartupBenchmark/0' (10 runs):
            :
            :       51317.845606      task-clock (msec)         #    2.133 CPUs 
utilized            ( +-  0.90% )
            :             13,214      context-switches          #    0.257 
K/sec                    ( +-  4.03% )
            :                292      cpu-migrations            #    0.006 
K/sec                    ( +-  1.76% )
            :            737,815      page-faults               #    0.014 
M/sec                    ( +-  1.49% )
            :    144,898,246,536      cycles                    #    2.824 GHz  
                    ( +-  1.08% )
            :    126,702,271,070      instructions              #    0.87  insn 
per cycle           ( +-  0.05% )
            :     24,116,649,584      branches                  #  469.947 
M/sec                    ( +-  0.05% )
            :        106,793,688      branch-misses             #    0.44% of 
all branches          ( +-  0.35% )
            :
            :       24.055824830 seconds time elapsed                           
               ( +-  0.89% )
            :
            : With deletes, the difference seems to decrease to about 14% when 
90% of
            : the blocks are deleted.
> If you've drawn conclusions or summarizations about these numbers, it'd be
Done


http://gerrit.cloudera.org:8080/#/c/17974/6//COMMIT_MSG@77
PS6, Line 77:  Performance counter stats for './bin/log_block_manager-test 
--gtest_filter=*StartupBenchmark/1' (10 runs):
> Probably worth looping a few times to see if the behavior holds across seve
Done


http://gerrit.cloudera.org:8080/#/c/17974/6//COMMIT_MSG@81
PS6, Line 81:                530      cpu-migrations            #    0.010 
K/sec                    ( +-  1.48% )
            :            399,284      page-faults               #    0.007 
M/sec                    ( +-  1.66% )
            :    145,147,457,046      cycles                    #    2.726 GHz  
                    ( +-  0.48% )
> Same here, any conclusions?
Done


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
I don't think there's a security concern here, but in the design I wrote 
everything but instance files would be encrypted, so after you pointing this 
out w.r.t. tablet metadata, I changed all these to sensitive as well. I don't 
think there's a benefit either way regarding these two file types.


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 is_sensitive) {
> nit: should be 'is_sensitive' too? Same elsewhere
Done


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: SensitivityMode sensitivi
> nit: should we introduce some Sensitivity enum for this or somesuch?
Done



--
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: 7
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, 10 Nov 2021 15:20:43 +0000
Gerrit-HasComments: Yes

Reply via email to