Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8352 )
Change subject: tool: new actions for adding and removing data directories ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/8352/2/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/8352/2/src/kudu/tools/kudu-tool-test.cc@2330 PS2, Line 2330: NO_FATALS(RunActionStdoutNone(Substitute( > Ah, good example. Yes, the assertion implies that it's past checking data d Interesting problem, hadn't thought of that.. To give a bit more context, TSTabletManager::Init() will load all of the tablet metadata from disk first, and once all are loaded, it will open the blocks. My guess is this is to prevent some disruptive IO patterns since this stresses the metadata directory. The loading is encapsulated by TabletMetadata::Load(), which verifies schemas, partitions, corruption, directory groups, etc. Once the metadata are successfully loaded, CreateAndRegisterTabletReplica() is called on each metadata object to create a replica. This call is used in a few places: creating a brand new replica, during tablet copies, when bootstrapping the server, and also when starting up and the tablet data is messed up, in which case a tombstoned replica is created (we may be able to do something similar! See HandleNonReadyTabletOnStartup() in ts_tablet_manager.cc). At first I was thinking we could somehow use the superblock to do some sort of group-size comparison, but it's tricky because TabletMetadata::Load abstracts away access to it. I suppose if we did want to defer resolution, one path forward would be to store the superblock in the tablet metadata when loading it from disk. Alternatively we could maybe push parts or all of OpenTabletMeta into CreateAndRegisterTabletReplica, although this would require a bit of refactoring in a few places since the calls aren't always paired together (e.g. in the case of tablet copies). We could also special-case this like we do for HandleNonReadyTabletOnStartup() and create a dummy replica that gets failed immediately. I think a relatively clean solution would be to wrap the metadata in a struct/tuple like TabletMetadataAndStatus that could be passed to CreateAndRegisterTabletReplica, which would then check the status if it's a "missing directory" error, and successfully create a failed replica. This would have the benefit of being usable in the case of disk failure as well, although at this point I think we should only permit the "missing directory" case. The tricky thing here is that TabletMetadata::Load() may return different versions of IOError, NotFound, etc., that we may not necessarily want to create a failed replica for (although maybe we should?). Based on description-length, I'm leaning towards the last solution, although I think it'd be a small amount of ugly plumbing. Trying to think of more, but from these, wdyt? -- To view, visit http://gerrit.cloudera.org:8080/8352 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddbdd6cc6231996e7802a622a8b4691527a0643 Gerrit-Change-Number: 8352 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Tue, 24 Oct 2017 01:41:17 +0000 Gerrit-HasComments: Yes
