Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14856 )
Change subject: fs: make some fields optional ...................................................................... Patch Set 2: Code-Review+1 (1 comment) 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: yes. > I wouldn't claim backward compatibility here since apparently it's not back To Andrew's point, it's important to remember the (narrow) way in which we currently use these PB messages: the DataDirManager loads them into memory, each from its own 'block_manager_instance' file found in a Kudu data directory. This is the "old reader with an old schema" that we have to be careful about breaking. There are no others. Even 'kudu pbc dump' isn't an "old reader" per se; it uses the schema information in the PBC file to figure out how to read the contents of the file. With that in mind, here's how we _could_ break backwards compatibility: 1. Mark these fields as optional. 2. Omit them from newly created block_manager_instance files written out to data directories. 3. Try to open those files using an older version of Kudu by loading the filesystem from disk (i.e. starting a server or running certain CLI commands). Andrew is staking the following position: the DataDirManager will _always_ include these fields when it writes out these messages, even though they're now going to be optional. Only the new WAL-based directory manager will omit the fields, and the instance files it creates are guaranteed NOT to be loaded by a DataDirManager, since they're going to be in the WAL directories rather than the data directories, and presumably they won't be called 'block_manager_instance' either. You can argue that Andrew can't guarantee this for all time. Maybe someone will slip up and start omitting the fields from new block_manager_instance files created by the DataDirManager. Maybe the WAL directory manager will somehow write block_manager_instance files in data directories. My take is that with decent documentation I think we can avoid both of these mistakes. -- 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: 2 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 06 Dec 2019 05:30:18 +0000 Gerrit-HasComments: Yes
