Adar Dembo has posted comments on this change. Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash ......................................................................
Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/4551/3/src/kudu/integration-tests/open-readonly-fs-itest.cc File src/kudu/integration-tests/open-readonly-fs-itest.cc: PS3, Line 54: // To repro KUDU-1657, many flushes need to be happening, so turn knobs in : // order to write and flush as fast as possible. Nit: that explains flags #2 and #3, but not #1. For #1, I think it's because you don't want the test to eat gobs of disk space right? Or was it because you wanted container file sizes to change as often as possible? PS3, Line 80: int num_columns = 50; : auto timeout = MonoDelta::FromSeconds(60); Nit: maybe declare these at the top of the test with kFoo syntax, so it's clear they're constants that control the performance characteristics of the test? Line 91: CHECK_OK(b.Build(&schema)); Nit: could be ASSERT_OK since it's in a test. -- To view, visit http://gerrit.cloudera.org:8080/4551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
