David Ribeiro Alves has posted comments on this change. Change subject: log block manager: corruptor test utility ......................................................................
Patch Set 3: (17 comments) http://gerrit.cloudera.org:8080/#/c/6582/3/src/kudu/fs/block_manager-stress-test.cc File src/kudu/fs/block_manager-stress-test.cc: PS3, Line 187: InjectNonFatalInconsistencies docs http://gerrit.cloudera.org:8080/#/c/6582/3/src/kudu/fs/log_block_manager-test-util.cc File src/kudu/fs/log_block_manager-test-util.cc: Line 136: nit: extra line PS3, Line 153: and metadata file in this case the metadata file exists right? can you make that clear? PS3, Line 154: Empty data file and metadata file both empty? PS3, Line 184: at least nit: redundant PS3, Line 213: // 0. No block offset. : // 1. No block length. : // 2. Negative block offset. : // 3. Negative block length. : // 4. Offset + length > data file size. : // 5. Two CREATEs for same block ID. : // 6. DELETE without first matching CREATE. : // 7. Unrecognized op type. are these based on stuff we've seen, or just the ways you think this might go wrong? PS3, Line 251: at least nit: redundant PS3, Line 320: Once done what does this refer to? PS3, Line 346: // Loop and try a different operation. maybe add a "continue" for clarity? (might seem like this comment is referring to the else below) http://gerrit.cloudera.org:8080/#/c/6582/3/src/kudu/fs/log_block_manager-test-util.h File src/kudu/fs/log_block_manager-test-util.h: PS3, Line 99: ContainerMode maybe "FindContainerMode"? http://gerrit.cloudera.org:8080/#/c/6582/3/src/kudu/fs/log_block_manager-test.cc File src/kudu/fs/log_block_manager-test.cc: PS3, Line 92: GetOnlyContainerDataFile docs PS3, Line 99: GetOnlyContainer docs PS3, Line 116: AssertEmptyReport should we add an AllOK() method to FsReport or something? PS3, Line 146: // Return what was requested. redundant PS3, Line 805: several variants of "incompleteness" at random. point to somewhere where we can find the possible incompleteness modes? PS3, Line 842: malformedness same PS3, Line 912: // Add some partial records. : LBMCorruptor corruptor(env_, { test_dir_ }, SeedRandom()); : ASSERT_OK(corruptor.Init()); : for (int i = 0; i < kNumRecords; i++) { : ASSERT_OK(corruptor.AddPartialRecordToContainer()); : } any way to assert that the errors were there without fixing? i.e. if you did nothing here, would the code below this still work? -- To view, visit http://gerrit.cloudera.org:8080/6582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I197b693d5a3c1a909e91b772ebfb31e50bae488b Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
