Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14856 )
Change subject: fs: mark 'filesystem_block_size_bytes' optional ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/14856/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14856/2//COMMIT_MSG@17 PS2, Line 17: lder > Adar, thank you for the explanation. I'm just trying to understand what th Thanks Adar. Right, I don't expect the now-optional fields to go away any time soon for data directories. And even for the WAL directory, I think most of the fields will remain (only 'filesystem_block_size_bytes', since I'm planning on renaming 'block_manager_type' to 'dir_type'). I made this change in the spirit of trying to avoid using required fields if possible, as recommended by protobuf. To your point about safety, I really can't guarantee complete safety with this change to optional beyond documentation. That said, I know I was a bit aggressive in switching over to optional. I don't strictly need all of them to be optional -- the aim was to do a bit of cleanup before using these messages more. I've cut back some of the switches, leaving the main one that I expect will actually not need to be used in the WAL directory manager. http://gerrit.cloudera.org:8080/#/c/14856/2//COMMIT_MSG@16 PS2, Line 16: nal shouldn't : pos > It's not only use, but populate all of them. How to guarantee that? And i Right, they're persisted to disk when creating new data directories. There isn't a strict guarantee other than making sure we fill them out. Generally that's what the 'required' tag was for, but it seems like protobuf docs warn against using 'required'. The absence of these fields would probably lead to a crash when starting up Kudu, whether these are marked required or not. -- To view, visit http://gerrit.cloudera.org:8080/14856 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5378391a94e5faa54a11a277d9191d151e0225d5 Gerrit-Change-Number: 14856 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 06 Dec 2019 07:39:10 +0000 Gerrit-HasComments: Yes
