Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9393 )
Change subject: KUDU-2290: Tool to re-create a tablet ...................................................................... Patch Set 5: (3 comments) I have reservations about how this patch introduces a new 'quarantined' table. In no other circumstances do we pollute the table namespace with auto-created names, and I don't think this is a good reason to start. This will become very painful down the line when begin to integrate with thirdparty metadata systems like Hive and Sentry which have no such concept as a quarantined table. I think it would be better to simply swap in the new tablet to the existing table, and provide the option to have the master not delete the tablet, if the data is still relevant. The data could then still be accessed and operated on via the CLI tools which take the tablet ID directly. Even then there are still authorization issues that will need to be figured out eventually. http://gerrit.cloudera.org:8080/#/c/9393/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9393/5//COMMIT_MSG@26 PS5, Line 26: eventually this will stop > Maybe be more specific; this behavior ends when the cached entries' TTL exp There may be some interesting fallout from this. In particular, I believe this is the first time in which the meta-cache TTL is significant for _positive_ cache locations . Right now I think the TTL only has observable effects for non-covered ranges. There's nothing actionable, but it's maybe something to think about and document. http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/client/meta_cache.cc@912 PS5, Line 912: // Partition should not have changed. > Could you clarify this a bit: the partition's _boundaries_ should have not What is a partition except a pair of bounds, in other words, what does your suggestion clarify? http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/9393/5/src/kudu/master/master.proto@715 PS5, Line 715: > I consider myself to be a protobuf luddite, but I don't understand why 'opt Keep it optional, please. We should never introduce new required fields. This is a great example, since you could later go back and add a variant of tablet replacement which just deletes the old version, and doesn't move it to a quarantined state. If we lived in a perfect world with perfect foresight we could use required, but we don't and we shouldn't. -- To view, visit http://gerrit.cloudera.org:8080/9393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifbefbde68e3ca724f04efe0426a3906e5c33ed3c Gerrit-Change-Number: 9393 Gerrit-PatchSet: 5 Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Tue, 13 Mar 2018 23:45:30 +0000 Gerrit-HasComments: Yes
