Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11259 )

Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11259/1/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/11259/1/src/kudu/fs/fs_manager.cc@625
PS1, Line 625: // Return true if 'fname' is a valid tablet ID.
> yeah, I agree it would be a better approach
I can update this method to use ObjectIdGenerator::Canonicalize to ensure the 
ids are valid. Currently we have only filtered theses special file types. This 
will technically be more strict. Is there a risk of breaking anything?


http://gerrit.cloudera.org:8080/#/c/11259/1/src/kudu/fs/fs_manager.cc@625
PS1, Line 625: // Return true if 'fname' is a valid tablet ID.
> Any idea why we don't implement this by verifying that the filename is a 32
I am not sure why. It was added here: 
https://github.com/apache/kudu/commit/dc20efa0b66c798f73aafdc694a3566317366fd6

I think the implementation is assuming everything but these exceptions must be 
valid. I agree, it would be better to filter on the real pattern.


http://gerrit.cloudera.org:8080/#/c/11259/1/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:

http://gerrit.cloudera.org:8080/#/c/11259/1/src/kudu/util/env_util.cc@312
PS1, Line 312:         iter->find(kTmpInfix) != string::npos ||
             :         iter->find(kBakInfix) != string::npos) {
> There are no tests today that will make a backup file. As far as temp files
I removed this change from the patch.



--
To view, visit http://gerrit.cloudera.org:8080/11259
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1
Gerrit-Change-Number: 11259
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <abu...@apache.org>
Gerrit-Reviewer: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 20 Aug 2018 16:13:00 +0000
Gerrit-HasComments: Yes

Reply via email to