You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2016/10/03 19:38:58 UTC
[kudu-CR](branch-1.0.x) KUDU-1090: relax MemTracker uniqueness constraint
Dan Burkert has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4601
Change subject: KUDU-1090: relax MemTracker uniqueness constraint
......................................................................
KUDU-1090: relax MemTracker uniqueness constraint
This patch relaxes the MemTracker uniqueness constraint to avoid certain
rare crashes (e.g. web UI takes a reference to a MemRowSet tracker during
Tablet::RewindSchemaForBootstrap). Details on these crashes can be found in
the associated bug report.
More specifically, the constraint is preserved, but it is only enforced
during FindTracker() or FindOrCreateTracker(). This way it is there for
MemTracker users that really should avoid duplicates (e.g. enforcing that
every tablet has just one MemTracker for all of its DeltaMemStores), but out
of the way for everyone else. With this change, we can remove the various
hacks and workarounds that were sprinkled in various tests.
Without the patch, the new test failed 2% of dist-test runs, though I could
not get it to fail locally at all.
I also removed a few test-only overrides of table creation timeout. They are
detrimental now that the default admin operation timeout is 30s (it used to
be 5s).
Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146
Reviewed-on: http://gerrit.cloudera.org:8080/4394
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
(cherry picked from commit 4b9d2f6976f45ea57e9a2c2648f31b3a0941a569)
---
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/ts_itest-base.h
A src/kudu/integration-tests/webserver-stress-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/util/mem_tracker-test.cc
M src/kudu/util/mem_tracker.cc
M src/kudu/util/mem_tracker.h
22 files changed, 176 insertions(+), 144 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/4601/1
--
To view, visit http://gerrit.cloudera.org:8080/4601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.0.x
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
[kudu-CR](branch-1.0.x) KUDU-1090: relax MemTracker uniqueness constraint
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1090: relax MemTracker uniqueness constraint
......................................................................
Patch Set 1: Code-Review+2 Verified+1
The failure was a known flake in client-test.
--
To view, visit http://gerrit.cloudera.org:8080/4601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.0.x
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR](branch-1.0.x) KUDU-1090: relax MemTracker uniqueness constraint
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: KUDU-1090: relax MemTracker uniqueness constraint
......................................................................
KUDU-1090: relax MemTracker uniqueness constraint
This patch relaxes the MemTracker uniqueness constraint to avoid certain
rare crashes (e.g. web UI takes a reference to a MemRowSet tracker during
Tablet::RewindSchemaForBootstrap). Details on these crashes can be found in
the associated bug report.
More specifically, the constraint is preserved, but it is only enforced
during FindTracker() or FindOrCreateTracker(). This way it is there for
MemTracker users that really should avoid duplicates (e.g. enforcing that
every tablet has just one MemTracker for all of its DeltaMemStores), but out
of the way for everyone else. With this change, we can remove the various
hacks and workarounds that were sprinkled in various tests.
Without the patch, the new test failed 2% of dist-test runs, though I could
not get it to fail locally at all.
I also removed a few test-only overrides of table creation timeout. They are
detrimental now that the default admin operation timeout is 30s (it used to
be 5s).
Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146
Reviewed-on: http://gerrit.cloudera.org:8080/4394
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
(cherry picked from commit 4b9d2f6976f45ea57e9a2c2648f31b3a0941a569)
Reviewed-on: http://gerrit.cloudera.org:8080/4601
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/ts_itest-base.h
A src/kudu/integration-tests/webserver-stress-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/util/mem_tracker-test.cc
M src/kudu/util/mem_tracker.cc
M src/kudu/util/mem_tracker.h
22 files changed, 176 insertions(+), 144 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/4601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iea8e3d383878e829188eaca65d639bb44d8b8146
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.0.x
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>