You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2019/10/16 19:27:12 UTC

[kudu-CR] kudu-tool-test: deflake TestFsAddRemoveDataDirEndToEnd

Hello Andrew Wong,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/14462

to review the following change.


Change subject: kudu-tool-test: deflake TestFsAddRemoveDataDirEndToEnd
......................................................................

kudu-tool-test: deflake TestFsAddRemoveDataDirEndToEnd

This test was made somewhat flaky by KUDU-2901. The reason is subtle: if any
unrelated IO (such as from a concurrently running test) interleaves with the
creation of the second table, it's possible for the new data directory to be
measured with less available space than the existing ones. The existing LBM
containers have all been preallocated by this point so the test never
remeasures available space. Thus, when the second table flushes, the
KUDU-2901 heuristic ensures that all new data blocks land on the existing
data directories rather than the new one.

The fix is simple: a feature flag for this feature (which we should have had
anyway), which we disable for the test.

Change-Id: Ifb92759318c12d791d26eb8fb6b77914c284f4cb
---
M src/kudu/fs/data_dirs.cc
M src/kudu/tools/kudu-tool-test.cc
2 files changed, 19 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/14462/1
-- 
To view, visit http://gerrit.cloudera.org:8080/14462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifb92759318c12d791d26eb8fb6b77914c284f4cb
Gerrit-Change-Number: 14462
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>

[kudu-CR] kudu-tool-test: deflake TestFsAddRemoveDataDirEndToEnd

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.

Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14462 )

Change subject: kudu-tool-test: deflake TestFsAddRemoveDataDirEndToEnd
......................................................................

Patch Set 1: Verified+1

(1 comment)

Overriding Jenkins, unrelated test failure.

http://gerrit.cloudera.org:8080/#/c/14462/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14462/1//COMMIT_MSG@18
PS1, Line 18: which we should have had
: anyway
> When is this the case? The new behavior doesn't necessarily seem like somet
Technically KUDU-2901 is a new feature for 1.11.0, and we generally roll out a new feature with a feature flag just in case it has unintended behavior. We'd then remove the flag in a subsequent release once we've established that the feature is safe.

Reasonable people can disagree on whether or not this feature is big/scary enough to warrant a flag. Given its systemic nature, I'd argue it deserves one. And regardless, it proves useful in situations like these.

--
To view, visit http://gerrit.cloudera.org:8080/14462
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb92759318c12d791d26eb8fb6b77914c284f4cb
Gerrit-Change-Number: 14462
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Oct 2019 20:45:59 +0000
Gerrit-HasComments: Yes