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

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


Patch Set 9: Code-Review+1

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/17974/7//COMMIT_MSG@133
PS7, Line 133: I also wanted to run dense_node-itest with -num_seconds=240 and
> It seems the results are all over the place even with rowset compaction dis
Yeah, I agree. It doesn't seem too useful to include.

Perhaps we can consider a similar test though, perhaps one that copies data 
from an encrypted server onto a non-encrypted server, and then bootstraps from 
the non-encrypted server (or vice versa). Though I'm fine leaving that for a 
later patch


http://gerrit.cloudera.org:8080/#/c/17974/9/src/kudu/tools/tool_action_pbc.cc
File src/kudu/tools/tool_action_pbc.cc:

http://gerrit.cloudera.org:8080/#/c/17974/9/src/kudu/tools/tool_action_pbc.cc@106
PS9, Line 106: IsFileEncrypted
nit: Is this actually meant to determine whether the file is sensitive or not?


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

PS9:
Do you think it's worth adding a test that checks that we can't read a 
sensitive file when encryption is disabled? Or that we can't read a sensitive 
unencrypted file (e.g. written when encryption was disabled) when encryption is 
enabled?

I suppose the error scenarios may change when you add the headers, but it's 
worth thinking about early what it looks like when we're looking at mismatched 
encryption statuses



--
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: 9
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: Fri, 17 Dec 2021 21:04:35 +0000
Gerrit-HasComments: Yes

Reply via email to