[kudu-CR] KUDU-1755 Part 2: Improve tablet on-disk size metric
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6968 to look at the new patch set (#16). Change subject: KUDU-1755 Part 2: Improve tablet on-disk size metric .. KUDU-1755 Part 2: Improve tablet on-disk size metric This adds WAL segment and cmeta sizes to the tablet on-disk metric. It also fixes the tablet superblock on disk size, which was being underestimated because the size was measured as the PB size instead of counting the container file the PB is stored in. It also exposes a second metric, Tablet Data Size On Disk, which measures just the size of data blocks and does not include metadata or WAL segments. Change-Id: Ia83f8aae0a544abbd19f5a34520b4828b89b6315 --- M src/kudu/consensus/consensus_meta-test.cc M src/kudu/consensus/consensus_meta.cc M src/kudu/consensus/consensus_meta.h M src/kudu/consensus/log-test.cc M src/kudu/consensus/log.cc M src/kudu/consensus/log.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/memrowset.h M src/kudu/tablet/mock-rowsets.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_metadata-test.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h 24 files changed, 277 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/6968/16 -- To view, visit http://gerrit.cloudera.org:8080/6968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia83f8aae0a544abbd19f5a34520b4828b89b6315 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-1755 Part 2: Improve tablet on-disk size metric
Will Berkeley has posted comments on this change. Change subject: KUDU-1755 Part 2: Improve tablet on-disk size metric .. Patch Set 15: (2 comments) http://gerrit.cloudera.org:8080/#/c/6968/13/src/kudu/consensus/consensus_meta.h File src/kudu/consensus/consensus_meta.h: PS13, Line 244: d to > That is a really cool post from Joshua Bloch. Anyway, we have to decide whe I choose int64_t. http://gerrit.cloudera.org:8080/#/c/6968/15/src/kudu/tablet/diskrowset.cc File src/kudu/tablet/diskrowset.cc: PS15, Line 736: base_data_->OnDiskDataSize() > I believe this should be OnDiskDataSize() Done -- To view, visit http://gerrit.cloudera.org:8080/6968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia83f8aae0a544abbd19f5a34520b4828b89b6315 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-1755 Part 2: Improve tablet on-disk size metric
Alexey Serbin has posted comments on this change. Change subject: KUDU-1755 Part 2: Improve tablet on-disk size metric .. Patch Set 15: (3 comments) http://gerrit.cloudera.org:8080/#/c/6968/16/src/kudu/consensus/log-test.cc File src/kudu/consensus/log-test.cc: PS16, Line 1166: ASSERT_EQ(anchors.size(), 3); nit: the expected value comes first http://gerrit.cloudera.org:8080/#/c/6968/15/src/kudu/consensus/log.cc File src/kudu/consensus/log.cc: PS15, Line 910: log_state_ == kLogClosed What happens with the log when it closes? Does it effective size go zero in that case? Would it make sense to cache the size of the log upon closing, so it would be available even in that case? Or this state is really ephemeral and we should not bother about that at all because a closed log is not going to hand around any longer? http://gerrit.cloudera.org:8080/#/c/6968/16/src/kudu/tablet/tablet_replica.h File src/kudu/tablet/tablet_replica.h: PS16, Line 24: #include nit: prefer for C++11 (I need to update IWYU stuff to give proper suggestions). -- To view, visit http://gerrit.cloudera.org:8080/6968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia83f8aae0a544abbd19f5a34520b4828b89b6315 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] Add a simple benchmark to create 1M blocks and reopen LBM
Adar Dembo has posted comments on this change.
Change subject: Add a simple benchmark to create 1M blocks and reopen LBM
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8006/1/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:
Line 793: ASSERT_OK_FAST(block->Finalize());
By finalizing each block, you'll fill up one container at a time, which
maximizes the number of blocks relative to the number of containers on disk.
Don't you want something more representative of a Kudu workflow, with multiple
outstanding transactions, each writing some group of blocks at a time? That'd
be equivalent to several concurrent flushes, and is similar to what
block_manager-stress-test does.
Line 799: LOG_TIMING(INFO, "reopening block manager") {
Maybe do the timing yourself and print out the average?
--
To view, visit http://gerrit.cloudera.org:8080/8006
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cc3547db7d8389c4e89ff9d4d3043b5f2fbe878
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] ref counted: fix move constructors to actually move
Adar Dembo has posted comments on this change. Change subject: ref_counted: fix move constructors to actually move .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/8002/2/src/kudu/gutil/ref_counted.h File src/kudu/gutil/ref_counted.h: PS2, Line 252: // Move constructor. This is required in addition to the conversion : // constructor below in order for clang to warn about pessimizing moves. After adding this, did you run clang-tidy across the codebase to see these warnings? Or are they emitted during regular compilation? -- To view, visit http://gerrit.cloudera.org:8080/8002 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b6b6753ab16221e065900eba5a7c178975d38a6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] log block manager: switch from google::sparse hash map to sparsepp
Adar Dembo has posted comments on this change.
Change subject: log_block_manager: switch from google::sparse_hash_map to
sparsepp
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/8007/1//COMMIT_MSG
Commit Message:
Line 14: This improved startup time 7-8x on a real host with ~11M blocks:
To be clear, this improvement is just from switching to sparsepp? Or with the
other optimizations rolled in?
http://gerrit.cloudera.org:8080/#/c/8007/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:
PS1, Line 328: SPARSEPP_PATCHLEVEL=0
: delete_if_wrong_patchlevel $SPARSEPP_SOURCE $SPARSEPP_PATCHLEVEL
: if [ ! -d "$SPARSEPP_SOURCE" ]; then
: fetch_and_expand sparsepp-${SPARSEPP_VERSION}.tar.gz
: pushd $SPARSEPP_SOURCE
: touch patchlevel-$SPARSEPP_PATCHLEVEL
: popd
: fi
If there's not one patch, you can omit the patchlevel-0 boilerplate, as well as
the call to delete_if_wrong_patchlevel.
http://gerrit.cloudera.org:8080/#/c/8007/1/thirdparty/vars.sh
File thirdparty/vars.sh:
Line 193: # (from https://github.com/toddlipcon/sparsepp)
Why are we pulling from your fork of sparsepp and not the main repo? Have you
patched it?
--
To view, visit http://gerrit.cloudera.org:8080/8007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] log block manager: switch from google::sparse hash map to sparsepp
Adar Dembo has posted comments on this change. Change subject: log_block_manager: switch from google::sparse_hash_map to sparsepp .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8007/1//COMMIT_MSG Commit Message: Forgot to ask: we originally switched to sparse_hash_map because of it's excellent memory footprint. Is sparsepp better, worse, or the same? -- To view, visit http://gerrit.cloudera.org:8080/8007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] log block manager: use move semantics to fill in the block map
Adar Dembo has posted comments on this change. Change subject: log_block_manager: use move semantics to fill in the block map .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/8008/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: PS1, Line 2210: UntrackedBlockMap::value_type Nit: change to auto, if you're interested. -- To view, visit http://gerrit.cloudera.org:8080/8008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8fe65087585e118f91320ec39f537b5f3ad6552f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Avoid a few allocations while reading PBC files
Adar Dembo has posted comments on this change.
Change subject: Avoid a few allocations while reading PBC files
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/8009/1/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
Line 367: Status DoReadV(int fd, const string& filename, uint64_t offset,
Could you make the same change to DoWriteV, so the two are consistent?
Line 755: virtual Status ReadV(uint64_t offset, vector* results) const
OVERRIDE {
The block manager data read paths go through ReadV(), not Read(), so they're
still paying the cost of using a vector. Shouldn't we also change these APIs to
use C-style arrays?
It may not affect LBM startup (since PBC file methods call Read(), AFAICT), but
it does affect bootstrapping.
http://gerrit.cloudera.org:8080/#/c/8009/1/src/kudu/util/pb_util.cc
File src/kudu/util/pb_util.cc:
Line 198:Slice* result, faststring* buf) {
The rationale to preferring faststring over a simpler memory buffer isn't
always obvious. Since the change is just to ValidateAndReadData here, perhaps
you can add a sentence or two in the comment to justify it?
--
To view, visit http://gerrit.cloudera.org:8080/8009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I228f26416b750c5a30ec6cc0763257c7d8b8d56f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Dan Burkert has posted comments on this change.
Change subject: KUDU-2124. Don't hold session lock while initializing a
TabletCopySession
..
Patch Set 3:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/7985/3/src/kudu/integration-tests/tablet_copy-itest.cc
File src/kudu/integration-tests/tablet_copy-itest.cc:
Line 1643: Status s = itest::BeginTabletCopySession(ts, tablet_id,
"dummy-uuid", kTimeout, &resp);
> Since the response isn't actually used, consider dropping it from BeginTabl
Done
Line 1674: ASSERT_LT(lat, kInjectedLatency);
> This seems risky; a variable amount of latency could be added to a failure
This check is pretty critical to ensuring no regressions. We could weaken it a
bit, but it would correspondingly weaken the test. I looped this 100 times to
make sure it's not flaky, and it passed 100%.
http://dist-test.cloudera.org/job?job_id=dan.1504897900.15603
http://gerrit.cloudera.org:8080/#/c/7985/3/src/kudu/tserver/tablet_copy_service-test.cc
File src/kudu/tserver/tablet_copy_service-test.cc:
Line 265: }
> If you expect some contention for a fixed amount of time, it might be usefu
I think it's valuable to have this pinging the methods as fast as possible in
order to suss out concurrency issues.
http://gerrit.cloudera.org:8080/#/c/7985/3/src/kudu/tserver/tablet_copy_service.cc
File src/kudu/tserver/tablet_copy_service.cc:
PS3, Line 185: SessionEntry* session_entry = FindOrNull(sessions_,
session_id);
: // Ensure that another interleaved thread has not already
removed the failed session.
: if (session_entry && session == session_entry->session) {
: sessions_.erase(session_id);
: }
> Can you add to the comment to explain the identity check? I imagine it's to
Done
Line 267: // The corresponding session is already in the process of
initializing.
> Can you use RPC_RETURN_NOT_OK here?
Done
http://gerrit.cloudera.org:8080/#/c/7985/3/src/kudu/tserver/tablet_copy_source_session.cc
File src/kudu/tserver/tablet_copy_source_session.cc:
Line 73: TAG_FLAG(tablet_copy_session_inject_latency_on_init_ms, unsafe);
> Make it hidden.
Done
Line 74: TAG_FLAG(tablet_copy_session_inject_latency_on_init_ms, runtime);
> I don't see the new test changing the value of this flag at runtime; can th
Done
PS3, Line 128: TRACE("Injecting $0ms of latency due to
--tablet_copy_session_inject_latency_on_init_ms",
: FLAGS_tablet_copy_session_inject_latency_on_init_ms);
> Is this useful to the tests that usei t?
I think this is a common patter, e.g.
https://github.com/apache/kudu/blob/master/src/kudu/tablet/transactions/write_transaction.cc#L153
--
To view, visit http://gerrit.cloudera.org:8080/7985
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7985 to look at the new patch set (#4). Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. KUDU-2124. Don't hold session lock while initializing a TabletCopySession This patch replaces the interior mutex in TabletCopySourceSession with a dynamic once, so that initialization can happen concurrently. This allows initialization to happen outside of the critical section in the tablet copy service. Implemented a regression test that injects latency into BeginTabletCopySession() calls. Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/tablet_copy_service-test.cc M src/kudu/tserver/tablet_copy_service.cc M src/kudu/tserver/tablet_copy_source_session.cc M src/kudu/tserver/tablet_copy_source_session.h M src/kudu/tserver/tablet_server-test-base.h M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_server_test_util.cc M src/kudu/tserver/tablet_server_test_util.h 11 files changed, 244 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/7985/4 -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] WIP: pb util: avoid repeated stat() calls reading files
Adar Dembo has posted comments on this change.
Change subject: WIP: pb_util: avoid repeated stat() calls reading files
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8010/1/src/kudu/util/pb_util.cc
File src/kudu/util/pb_util.cc:
PS1, Line 260: if (!*cached_file_size) {
: RETURN_NOT_OK(reader->Size(&file_size));
: *cached_file_size = file_size;
: } else {
: file_size = cached_file_size->get();
: }
Nit: invert for clarity?
Line 355: RETURN_NOT_OK(reader->Size(&file_size));
What about this call?
--
To view, visit http://gerrit.cloudera.org:8080/8010
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I27371800604bcb20bafae7946d3b3e84af094598
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Avoid a few allocations while reading PBC files
Adar Dembo has posted comments on this change. Change subject: Avoid a few allocations while reading PBC files .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8009/1/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 573: return DoReadV(fd_, filename_, offset, &(*results)[0], results->size()); BTW, isn't results->data() a cleaner way to get at the vector's backing buffer? -- To view, visit http://gerrit.cloudera.org:8080/8009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I228f26416b750c5a30ec6cc0763257c7d8b8d56f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] catalog manager: various boring cleanup
Dan Burkert has posted comments on this change.
Change subject: catalog_manager: various boring cleanup
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7990/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 3766: committer_out.AddTablets({ new_tablet });
std::move(new_tablet)
--
To view, visit http://gerrit.cloudera.org:8080/7990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I028611361ae7d5ce2818707c203c045dbce294c6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Adar Dembo has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 4: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/7985/3/src/kudu/integration-tests/tablet_copy-itest.cc File src/kudu/integration-tests/tablet_copy-itest.cc: Line 1674: } > This check is pretty critical to ensuring no regressions. We could weaken Can you also try looping it 1000 times in TSAN mode with some stress threads? If that passes then I think it's OK. -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] catalog manager: various boring cleanup
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7990 to look at the new patch set (#3). Change subject: catalog_manager: various boring cleanup .. catalog_manager: various boring cleanup Change-Id: I028611361ae7d5ce2818707c203c045dbce294c6 --- M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h 2 files changed, 51 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/7990/3 -- To view, visit http://gerrit.cloudera.org:8080/7990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I028611361ae7d5ce2818707c203c045dbce294c6 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] catalog manager: various boring cleanup
Adar Dembo has posted comments on this change.
Change subject: catalog_manager: various boring cleanup
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7990/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 3766: committer_out.AddTablets({ new_tablet });
> std::move(new_tablet)
Done
--
To view, visit http://gerrit.cloudera.org:8080/7990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I028611361ae7d5ce2818707c203c045dbce294c6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Dan Burkert has posted comments on this change. Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. Patch Set 1: yikes. does this indicate a lack of test coverage around concurrent alter and tablet drops? The lack of read locks when creating tablets seems benign since those won't be visible, but the concurrent increment schema / decrement schema that could happen from dropping and altering a tablet at once seems like a big race. -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Dan Burkert has posted comments on this change. Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. Patch Set 1: Code looks good. -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] catalog manager: various boring cleanup
Dan Burkert has posted comments on this change. Change subject: catalog_manager: various boring cleanup .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7990/3/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: Line 2489: const string delete_msg = report_opid_index == consensus::kInvalidOpIdIndex ? Is this a bug fix? Seems the behavior is slightly different. -- To view, visit http://gerrit.cloudera.org:8080/7990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I028611361ae7d5ce2818707c203c045dbce294c6 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2137: protect against concurrent schema version change and tablet drop
Dan Burkert has posted comments on this change. Change subject: KUDU-2137: protect against concurrent schema version change and tablet drop .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7996/2/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: Line 4318: // locks are acquired before spinlocks. Perhaps you could grab the partition info before taking the spin locks? Should be OK to copy it (just a string). The partition info for a tablet is immutable. -- To view, visit http://gerrit.cloudera.org:8080/7996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I371fc310a97ae94ec2ebf04405db99c5f2937e1a Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2137: protect against concurrent schema version change and tablet drop
Dan Burkert has posted comments on this change.
Change subject: KUDU-2137: protect against concurrent schema version change and
tablet drop
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7996/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 4318: // locks are acquired before spinlocks.
> Perhaps you could grab the partition info before taking the spin locks? Sh
To be clear, something like this:
string partition_key_start;
{
TabletMetadataLock l(this, TabletMetadataLock::READ);
key_start = metadata().state().pb.partition().partition_key_start();
}
std::lock_guard table_l(table_->lock_);
std::lock_guard tablet_l(lock_);
...
--
To view, visit http://gerrit.cloudera.org:8080/7996
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I371fc310a97ae94ec2ebf04405db99c5f2937e1a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] ref counted: fix move constructors to actually move
Dan Burkert has posted comments on this change. Change subject: ref_counted: fix move constructors to actually move .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8002 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b6b6753ab16221e065900eba5a7c178975d38a6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Adar Dembo has posted comments on this change. Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. Patch Set 2: > yikes. does this indicate a lack of test coverage around > concurrent alter and tablet drops? The lack of read locks when > creating tablets seems benign since those won't be visible, but the > concurrent increment schema / decrement schema that could happen > from dropping and altering a tablet at once seems like a big race. Not sure I follow the race; aren't the increment/decrement schema operations protected by the table spinlock? The tablet's reported schema version is in-memory only so it's not protected by the tablet metadata lock, just by its spinlock (and the table spinlock, for the subsequent increment/decrement operations). -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-2137: protect against concurrent schema version change and tablet drop
Adar Dembo has posted comments on this change. Change subject: KUDU-2137: protect against concurrent schema version change and tablet drop .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7996/2/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: Line 4318: // locks are acquired before spinlocks. > To be clear, something like this: Great idea. Will make that change. -- To view, visit http://gerrit.cloudera.org:8080/7996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I371fc310a97ae94ec2ebf04405db99c5f2937e1a Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Avoid a few allocations while reading PBC files
Dan Burkert has posted comments on this change. Change subject: Avoid a few allocations while reading PBC files .. Patch Set 1: I don't want to block merging this, but we might want to think about this is a bigger context. This is an issue in a lot of APIs, and the fact of the matter is the c++ stdlib is not equipped to deal with it. It may be time to bring in something like gsl for the Span type https://github.com/microsoft/gsl. -- To view, visit http://gerrit.cloudera.org:8080/8009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I228f26416b750c5a30ec6cc0763257c7d8b8d56f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] catalog manager: various boring cleanup
Adar Dembo has posted comments on this change. Change subject: catalog_manager: various boring cleanup .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7990/3/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: Line 2489: const string delete_msg = report_opid_index == consensus::kInvalidOpIdIndex ? > Is this a bug fix? Seems the behavior is slightly different. That's https://github.com/apache/kudu/commit/cd6eee7a375e053f1f9bed338e603bf9ca91e709; I merely rebase on top of it. If you look at the Base-->PS3 diff, you won't see any changes here. I don't actually know the answer to your question; best to ask Andrew or Mike. -- To view, visit http://gerrit.cloudera.org:8080/7990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I028611361ae7d5ce2818707c203c045dbce294c6 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Avoid a few allocations while reading PBC files
Dan Burkert has posted comments on this change. Change subject: Avoid a few allocations while reading PBC files .. Patch Set 1: gsl::span : std::vector as Slice (or gsl::string_span) : std::string -- To view, visit http://gerrit.cloudera.org:8080/8009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I228f26416b750c5a30ec6cc0763257c7d8b8d56f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] KUDU-2137: protect against concurrent schema version change and tablet drop
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7996 to look at the new patch set (#3). Change subject: KUDU-2137: protect against concurrent schema version change and tablet drop .. KUDU-2137: protect against concurrent schema version change and tablet drop Try as I might, I could not reproduce the failure in the bug report. I looped the failed test several thousand times. I also looped the entire alter_table-test suite a thousand times. Finally, I wrote a unit test that hammers one table with concurrent add column, add partition, and drop partition operations. Nothing worked. So, here's my best guess at what's going on: if a tablet is dropped while the master is processing its report, it's conceivable that we could wind up in TabletInfo::set_reported_schema_version() with the table spinlock held just after TableInfo::AddRemoveTablets() dropped the tablet. This would cause us to decrement the tablet's "old" schema version from the table's count map twice: once when dropping the tablet and a second time in set_reported_schema_version(). The fix is straight-forward: after acquiring both spinlocks, double check that the tablet is still a member of the table. We need to take the tablet metadata lock to do this, but the saving grace is that tablet partition keys are immutable, so this lock acquisition needn't overlap with the spinlocks. Change-Id: I371fc310a97ae94ec2ebf04405db99c5f2937e1a --- M src/kudu/master/catalog_manager.cc 1 file changed, 21 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/7996/3 -- To view, visit http://gerrit.cloudera.org:8080/7996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I371fc310a97ae94ec2ebf04405db99c5f2937e1a Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Dan Burkert has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7985/3/src/kudu/integration-tests/tablet_copy-itest.cc File src/kudu/integration-tests/tablet_copy-itest.cc: Line 1674: ASSERT_LT(lat, kInjectedLatency); > Can you also try looping it 1000 times in TSAN mode with some stress thread done: http://dist-test.cloudera.org/job?job_id=dan.1504902130.6748 -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] KUDU-2137: protect against concurrent schema version change and tablet drop
Dan Burkert has posted comments on this change. Change subject: KUDU-2137: protect against concurrent schema version change and tablet drop .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I371fc310a97ae94ec2ebf04405db99c5f2937e1a Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Dan Burkert has posted comments on this change. Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Dan Burkert has posted comments on this change. Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. Patch Set 2: gotcha; sounds good. -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] catalog manager: various boring cleanup
Dan Burkert has posted comments on this change. Change subject: catalog_manager: various boring cleanup .. Patch Set 3: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/7990/3/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: Line 2489: const string delete_msg = report_opid_index == consensus::kInvalidOpIdIndex ? > That's https://github.com/apache/kudu/commit/cd6eee7a375e053f1f9bed338e603b oh sorry, should have recognized that. -- To view, visit http://gerrit.cloudera.org:8080/7990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I028611361ae7d5ce2818707c203c045dbce294c6 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Alexey Serbin has posted comments on this change. Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7995/2/src/kudu/util/cow_object.h File src/kudu/util/cow_object.h: PS2, Line 60: DCHECK(lock_.HasWriteLock()); Why not to put that check right into WriteUnlock()/CommitUnlock()/UpgradeToCommitLock() ? -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7985 to look at the new patch set (#5). Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. KUDU-2124. Don't hold session lock while initializing a TabletCopySession This patch replaces the interior mutex in TabletCopySourceSession with a dynamic once, so that initialization can happen concurrently. This allows initialization to happen outside of the critical section in the tablet copy service. Implemented a regression test that injects latency into BeginTabletCopySession() calls. Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/tablet_copy_service-test.cc M src/kudu/tserver/tablet_copy_service.cc M src/kudu/tserver/tablet_copy_source_session.cc M src/kudu/tserver/tablet_copy_source_session.h M src/kudu/tserver/tablet_server-test-base.h M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_server_test_util.cc M src/kudu/tserver/tablet_server_test_util.h 11 files changed, 243 insertions(+), 63 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/7985/5 -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Alexey Serbin has posted comments on this change.
Change subject: catalog_manager: fix unprotected data access in
TableInfo::AddRemoveTablets
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7995/2/src/kudu/master/catalog_manager-test.cc
File src/kudu/master/catalog_manager-test.cc:
PS2, Line 71: tablets.push_back(tablet);
If the call to table->AddRemoveTablets({}, tablets) at line 93 has gone, do we
need populating the tablets at all?
>From the other side, if removing that call at line 93, do we have some other
>place to cover that scenario?
--
To view, visit http://gerrit.cloudera.org:8080/7995
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Adar Dembo has posted comments on this change.
Change subject: catalog_manager: fix unprotected data access in
TableInfo::AddRemoveTablets
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7995/2/src/kudu/master/catalog_manager-test.cc
File src/kudu/master/catalog_manager-test.cc:
PS2, Line 71: tablets.push_back(tablet);
> If the call to table->AddRemoveTablets({}, tablets) at line 93 has gone, do
Bear in mind that the TableInfo only has raw pointers to its TabletInfos. The
expectation is that the CatalogManager stores strong refs to the TabletInfos in
its maps; 'tablets' serves as a stand-in for those maps in this test.
AFAICT, this is just test setup, so the AddRemoveTablets() call that dropped
the tablets wasn't providing any value.
http://gerrit.cloudera.org:8080/#/c/7995/2/src/kudu/util/cow_object.h
File src/kudu/util/cow_object.h:
PS2, Line 60: DCHECK(lock_.HasWriteLock());
> Why not to put that check right into WriteUnlock()/CommitUnlock()/UpgradeTo
Good idea. I've added those checks into RWCLock itself, though I'm keeping
these checks here because I think they're still useful (with the possibly
exception of this particular check, though consistency bears out).
--
To view, visit http://gerrit.cloudera.org:8080/7995
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Adar Dembo has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Dan Burkert has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 5: Verified+1 ignoring iwyu failure; looks wrong to me. -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] catalog manager: various boring cleanup
Adar Dembo has submitted this change and it was merged. Change subject: catalog_manager: various boring cleanup .. catalog_manager: various boring cleanup Change-Id: I028611361ae7d5ce2818707c203c045dbce294c6 Reviewed-on: http://gerrit.cloudera.org:8080/7990 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h 2 files changed, 51 insertions(+), 37 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I028611361ae7d5ce2818707c203c045dbce294c6 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Hello Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7995 to look at the new patch set (#3). Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets RWCLock::HasWriteLock never worked properly because last_writer_tid_ wasn't reset when the lock was released. Well, it worked properly (though it's far less useful) in non-DEBUG builds, but then the various HasWriteLock DCHECKs are compiled out. Who knew? When fixed, TableInfo::AddRemoveTablets broke; we had been reading some tablet metadata without first acquiring a lock! This was frustrasting to address because it meant retreading over the most error-prone aspect of the catalog manager: for operations that require several writes in order to "publish" their results, in what order should those writes occur? I tried my best to get this right, but who knows how I did... Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 --- M src/kudu/master/catalog_manager-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/util/cow_object.h M src/kudu/util/rwc_lock.cc M src/kudu/util/rwc_lock.h 6 files changed, 95 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/7995/3 -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Hello Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7995 to look at the new patch set (#4). Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets RWCLock::HasWriteLock never worked properly because last_writer_tid_ wasn't reset when the lock was released. Well, it worked properly (though it's far less useful) in non-DEBUG builds, but then the various HasWriteLock DCHECKs are compiled out. Who knew? When fixed, TableInfo::AddRemoveTablets broke; we had been reading some tablet metadata without first acquiring a lock! This was frustrasting to address because it meant retreading over the most error-prone aspect of the catalog manager: for operations that require several writes in order to "publish" their results, in what order should those writes occur? I tried my best to get this right, but who knows how I did... Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 --- M src/kudu/master/catalog_manager-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/util/cow_object.h M src/kudu/util/rwc_lock.cc M src/kudu/util/rwc_lock.h 6 files changed, 96 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/7995/4 -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR](gh-pages) Update website for 1.5.0 release
Hello Jean-Daniel Cryans, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/8013 to review the following change. Change subject: Update website for 1.5.0 release .. Update website for 1.5.0 release Change-Id: Id8885cd6ecfb904e95370e566a32a579f6c85a4a --- M apidocs/allclasses-frame.html M apidocs/allclasses-noframe.html M apidocs/constant-values.html M apidocs/deprecated-list.html M apidocs/help-doc.html M apidocs/index-all.html M apidocs/index.html M apidocs/org/apache/kudu/ColumnSchema.html M apidocs/org/apache/kudu/Schema.html M apidocs/org/apache/kudu/Type.html D apidocs/org/apache/kudu/annotations/InterfaceAudience.html D apidocs/org/apache/kudu/annotations/InterfaceStability.html D apidocs/org/apache/kudu/annotations/class-use/InterfaceAudience.html D apidocs/org/apache/kudu/annotations/class-use/InterfaceStability.html D apidocs/org/apache/kudu/annotations/package-frame.html D apidocs/org/apache/kudu/annotations/package-summary.html D apidocs/org/apache/kudu/annotations/package-tree.html D apidocs/org/apache/kudu/annotations/package-use.html M apidocs/org/apache/kudu/class-use/ColumnSchema.html M apidocs/org/apache/kudu/class-use/Schema.html M apidocs/org/apache/kudu/class-use/Type.html M apidocs/org/apache/kudu/client/AbstractKuduScannerBuilder.html M apidocs/org/apache/kudu/client/AlterTableOptions.html M apidocs/org/apache/kudu/client/AlterTableResponse.html M apidocs/org/apache/kudu/client/AsyncKuduClient.AsyncKuduClientBuilder.html M apidocs/org/apache/kudu/client/AsyncKuduClient.html M apidocs/org/apache/kudu/client/AsyncKuduScanner.AsyncKuduScannerBuilder.html M apidocs/org/apache/kudu/client/AsyncKuduScanner.ReadMode.html M apidocs/org/apache/kudu/client/AsyncKuduScanner.html M apidocs/org/apache/kudu/client/AsyncKuduSession.html M apidocs/org/apache/kudu/client/ColumnRangePredicate.html M apidocs/org/apache/kudu/client/CreateTableOptions.html M apidocs/org/apache/kudu/client/Delete.html M apidocs/org/apache/kudu/client/DeleteTableResponse.html M apidocs/org/apache/kudu/client/ExternalConsistencyMode.html M apidocs/org/apache/kudu/client/HasFailedRpcException.html M apidocs/org/apache/kudu/client/Insert.html M apidocs/org/apache/kudu/client/IsAlterTableDoneResponse.html M apidocs/org/apache/kudu/client/KuduClient.KuduClientBuilder.html M apidocs/org/apache/kudu/client/KuduClient.html M apidocs/org/apache/kudu/client/KuduException.html M apidocs/org/apache/kudu/client/KuduPredicate.ComparisonOp.html M apidocs/org/apache/kudu/client/KuduPredicate.html M apidocs/org/apache/kudu/client/KuduScanToken.KuduScanTokenBuilder.html M apidocs/org/apache/kudu/client/KuduScanToken.html M apidocs/org/apache/kudu/client/KuduScanner.KuduScannerBuilder.html M apidocs/org/apache/kudu/client/KuduScanner.html M apidocs/org/apache/kudu/client/KuduSession.html M apidocs/org/apache/kudu/client/KuduTable.html M apidocs/org/apache/kudu/client/ListTablesResponse.html M apidocs/org/apache/kudu/client/ListTabletServersResponse.html M apidocs/org/apache/kudu/client/LocatedTablet.Replica.html M apidocs/org/apache/kudu/client/LocatedTablet.html M apidocs/org/apache/kudu/client/Operation.html M apidocs/org/apache/kudu/client/OperationResponse.html M apidocs/org/apache/kudu/client/PartialRow.html M apidocs/org/apache/kudu/client/PleaseThrottleException.html M apidocs/org/apache/kudu/client/RangePartitionBound.html M apidocs/org/apache/kudu/client/ReplicaSelection.html M apidocs/org/apache/kudu/client/RowError.html M apidocs/org/apache/kudu/client/RowErrorsAndOverflowStatus.html M apidocs/org/apache/kudu/client/RowResult.html M apidocs/org/apache/kudu/client/RowResultIterator.html M apidocs/org/apache/kudu/client/SessionConfiguration.FlushMode.html M apidocs/org/apache/kudu/client/SessionConfiguration.html M apidocs/org/apache/kudu/client/Statistics.Statistic.html M apidocs/org/apache/kudu/client/Statistics.html M apidocs/org/apache/kudu/client/Status.html M apidocs/org/apache/kudu/client/Update.html M apidocs/org/apache/kudu/client/Upsert.html M apidocs/org/apache/kudu/client/class-use/AbstractKuduScannerBuilder.html M apidocs/org/apache/kudu/client/class-use/AlterTableOptions.html M apidocs/org/apache/kudu/client/class-use/AlterTableResponse.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduClient.AsyncKuduClientBuilder.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduClient.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.AsyncKuduScannerBuilder.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.ReadMode.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduSession.html M apidocs/org/apache/kudu/client/class-use/ColumnRangePredicate.html M apidocs/org/apache/kudu/client/class-use/CreateTableOptions.html M apidocs/org/apache/kudu/client/class-use/Delete.html M apidocs/org/apache/kudu/client/cl
[kudu-CR] KUDU-2137: protect against concurrent schema version change and tablet drop
Dan Burkert has posted comments on this change. Change subject: KUDU-2137: protect against concurrent schema version change and tablet drop .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I371fc310a97ae94ec2ebf04405db99c5f2937e1a Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Dan Burkert has posted comments on this change. Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-2137: protect against concurrent schema version change and tablet drop
Alexey Serbin has posted comments on this change. Change subject: KUDU-2137: protect against concurrent schema version change and tablet drop .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I371fc310a97ae94ec2ebf04405db99c5f2937e1a Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Alexey Serbin has posted comments on this change. Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/7995/2/src/kudu/master/catalog_manager-test.cc File src/kudu/master/catalog_manager-test.cc: PS2, Line 71: tablets.push_back(tablet); > Bear in mind that the TableInfo only has raw pointers to its TabletInfos. T I see -- thanks for the clarification. -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] catalog manager: fix unprotected data access in TableInfo::AddRemoveTablets
Adar Dembo has submitted this change and it was merged. Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets .. catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets RWCLock::HasWriteLock never worked properly because last_writer_tid_ wasn't reset when the lock was released. Well, it worked properly (though it's far less useful) in non-DEBUG builds, but then the various HasWriteLock DCHECKs are compiled out. Who knew? When fixed, TableInfo::AddRemoveTablets broke; we had been reading some tablet metadata without first acquiring a lock! This was frustrasting to address because it meant retreading over the most error-prone aspect of the catalog manager: for operations that require several writes in order to "publish" their results, in what order should those writes occur? I tried my best to get this right, but who knows how I did... Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Reviewed-on: http://gerrit.cloudera.org:8080/7995 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert Reviewed-by: Alexey Serbin --- M src/kudu/master/catalog_manager-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/util/cow_object.h M src/kudu/util/rwc_lock.cc M src/kudu/util/rwc_lock.h 6 files changed, 96 insertions(+), 22 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2137: protect against concurrent schema version change and tablet drop
Adar Dembo has submitted this change and it was merged. Change subject: KUDU-2137: protect against concurrent schema version change and tablet drop .. KUDU-2137: protect against concurrent schema version change and tablet drop Try as I might, I could not reproduce the failure in the bug report. I looped the failed test several thousand times. I also looped the entire alter_table-test suite a thousand times. Finally, I wrote a unit test that hammers one table with concurrent add column, add partition, and drop partition operations. Nothing worked. So, here's my best guess at what's going on: if a tablet is dropped while the master is processing its report, it's conceivable that we could wind up in TabletInfo::set_reported_schema_version() with the table spinlock held just after TableInfo::AddRemoveTablets() dropped the tablet. This would cause us to decrement the tablet's "old" schema version from the table's count map twice: once when dropping the tablet and a second time in set_reported_schema_version(). The fix is straight-forward: after acquiring both spinlocks, double check that the tablet is still a member of the table. We need to take the tablet metadata lock to do this, but the saving grace is that tablet partition keys are immutable, so this lock acquisition needn't overlap with the spinlocks. Change-Id: I371fc310a97ae94ec2ebf04405db99c5f2937e1a Reviewed-on: http://gerrit.cloudera.org:8080/7996 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert Reviewed-by: Alexey Serbin --- M src/kudu/master/catalog_manager.cc 1 file changed, 21 insertions(+), 2 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I371fc310a97ae94ec2ebf04405db99c5f2937e1a Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Hello Mike Percy, Adar Dembo, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/8016 to review the following change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. KUDU-2125: Tablet copy client does not retry on failures Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_copy_client_session-itest.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h 5 files changed, 125 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/8016/1 -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [tests] de-flaking catalog manager tsk-itest
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/8017 Change subject: [tests] de-flaking catalog_manager_tsk-itest .. [tests] de-flaking catalog_manager_tsk-itest After recent updates the catalog_manager_tsk-itest became unstable. One of the failure scenarios is when the test tablet server cannot register in the cluster within the specified timeout (30 seconds). It seems some test machines are too slow to accommodate the test scenario with 16ms Raft heartbeat interval. When running the test with too short Raft heartbeat interval, the following scenario occurs on slow or very busy machines: * An election happens among masters (e.g., term 1) and leader master is elected. * Shortly after that, the followers stop receiving some Raft heartbeats from the leader within the specified timeout interval. * The followers start new election, but experience timeouts for vote requests among them as well. * The leader fails getting responses from the followers for its UpdateConsensus RPC requests. * The tablet server fails to register with the cluster. Sometimes the scenario above is enriched with dropped incoming Raft requests due to the backpressure on the Raft RPC service queue in masters. The following changes where made to address the flakiness due to the described scenarios: * increasing the Raft heartbeat interval * increasing max length of the Raft RPC service queue * increasing the back-off interval after leader election failures After making the changes above, the test became more stable. Not a single failure was spot in multiple 1K runs when running by dist-test with --stress_cpu_threads=16: ASAN: http://dist-test.cloudera.org/job?job_id=aserbin.1504914035.18113 DEBUG: http://dist-test.cloudera.org/job?job_id=aserbin.1504911962.26895 RELEASE: http://dist-test.cloudera.org/job?job_id=aserbin.1504913524.8185 TSAN: http://dist-test.cloudera.org/job?job_id=aserbin.1504903775.17126 This is a follow-up for faa0b14effb6e15f9989d686e5a1f8e1040a1dd6. Change-Id: I50cee27a579cffa7232137c7039b02a1ad4ab7eb --- M src/kudu/integration-tests/catalog_manager_tsk-itest.cc 1 file changed, 5 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/8017/1 -- To view, visit http://gerrit.cloudera.org:8080/8017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I50cee27a579cffa7232137c7039b02a1ad4ab7eb Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8016 to look at the new patch set (#2). Change subject: KUDU-2125: Tablet copy client does not retry on failures .. KUDU-2125: Tablet copy client does not retry on failures Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_copy_client_session-itest.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h 5 files changed, 123 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/8016/2 -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Dan Burkert has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 6: Verified+1 ignore bogus iwyu -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Dan Burkert has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 2: Verified+1 bogus iwyu -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [tests] de-flaking catalog manager tsk-itest
Adar Dembo has posted comments on this change. Change subject: [tests] de-flaking catalog_manager_tsk-itest .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/8017/1//COMMIT_MSG Commit Message: Did you try looping this test with the recent failure-detection change (https://github.com/apache/kudu/commit/21b0f3d5e255760535e281efe5879fe657df1f1c) reverted? Todd suspected that it made elections converge more slowly, and it could be responsible for the flakiness here too. If it is responsible, it'd be better to address that than to continually tweak the test values ever so slightly to ensure it passes. http://gerrit.cloudera.org:8080/#/c/8017/1/src/kudu/integration-tests/catalog_manager_tsk-itest.cc File src/kudu/integration-tests/catalog_manager_tsk-itest.cc: Line 64: hb_interval_ms_(128), I get the feeling that, although these values may now be carefully tuned so that the test passes, that may change as Kudu continues to evolve, at which point we'll burn more time on deflaking it. Why exactly does this test need so many overridden values? Are they required for the test in some way? Or can any be reverted back to the defaults? -- To view, visit http://gerrit.cloudera.org:8080/8017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I50cee27a579cffa7232137c7039b02a1ad4ab7eb Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Adar Dembo has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [tests] de-flaking catalog manager tsk-itest
Alexey Serbin has posted comments on this change. Change subject: [tests] de-flaking catalog_manager_tsk-itest .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/8017/1//COMMIT_MSG Commit Message: > Did you try looping this test with the recent failure-detection change (htt I didn't try but I know it was not that flaky prior to that patch. I can double-check and report on that. Actually, I suspect there were 2 changelists that made this test flakier: this one and another one committed between 2 and 4 of September. I can dig in to find the exact ones. http://gerrit.cloudera.org:8080/#/c/8017/1/src/kudu/integration-tests/catalog_manager_tsk-itest.cc File src/kudu/integration-tests/catalog_manager_tsk-itest.cc: Line 64: hb_interval_ms_(128), > I get the feeling that, although these values may now be carefully tuned so In this test we want to induce many elections among masters, so that elections happen while a leader tries to write some data into the system catalog table (particularly, a new token signing key). Adding that --catalog_manager_inject_latency_prior_tsk_write_ms=1000 flag and making the Raft HB interval less than that 1000ms interval (along with disabling pre-elections) gives us the desired behavior. -- To view, visit http://gerrit.cloudera.org:8080/8017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I50cee27a579cffa7232137c7039b02a1ad4ab7eb Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Adar Dembo has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/8016/2/src/kudu/integration-tests/cluster_itest_util.h File src/kudu/integration-tests/cluster_itest_util.h: PS2, Line 330: running Nit: maybe 'only_running'? http://gerrit.cloudera.org:8080/#/c/8016/2/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: PS2, Line 277: // Make the service queue artificially short on the destination server so that : // ERROR_SERVER_TOO_BUSY is likely to occur. Misplaced comment? Line 296: ASSERT_OK(WaitForNumTabletsOnTS(ts0, kNumTablets, kDefaultTimeout, &tablets, true)); Why is running=true important for this test? -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [tests] de-flaking catalog manager tsk-itest
Alexey Serbin has posted comments on this change. Change subject: [tests] de-flaking catalog_manager_tsk-itest .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8017/1/src/kudu/integration-tests/catalog_manager_tsk-itest.cc File src/kudu/integration-tests/catalog_manager_tsk-itest.cc: Line 64: hb_interval_ms_(128), > In this test we want to induce many elections among masters, so that electi And the more re-election we have among masters, the better. That's why the Raft HB interval is set to those just tens/hundreds of milliseconds. Another approach might be setting --catalog_manager_inject_latency_prior_tsk_write_ms=1 and using the default Raft HB interval of 1 second, but that would require longer test runtime to get the same number of master re-elections during the test. -- To view, visit http://gerrit.cloudera.org:8080/8017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I50cee27a579cffa7232137c7039b02a1ad4ab7eb Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Dan Burkert has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/8016/2/src/kudu/integration-tests/cluster_itest_util.h File src/kudu/integration-tests/cluster_itest_util.h: PS2, Line 330: running > Nit: maybe 'only_running'? Done http://gerrit.cloudera.org:8080/#/c/8016/2/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: PS2, Line 277: // Make the service queue artificially short on the destination server so that : // ERROR_SERVER_TOO_BUSY is likely to occur. > Misplaced comment? Done Line 296: ASSERT_OK(WaitForNumTabletsOnTS(ts0, kNumTablets, kDefaultTimeout, &tablets, true)); > Why is running=true important for this test? Attempting to tablet copy a non-running tablet will fail, and that's not considered a retriable error. -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Dan Burkert has uploaded a new patch set (#3). Change subject: KUDU-2125: Tablet copy client does not retry on failures .. KUDU-2125: Tablet copy client does not retry on failures Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_copy_client_session-itest.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h 5 files changed, 121 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/8016/3 -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Adar Dembo has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/8016/2/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: Line 296: > Attempting to tablet copy a non-running tablet will fail, and that's not co I understand, but I don't see any code here to delete or otherwise fail tablets, so shouldn't all tablets post-restart basically be running? -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Dan Burkert has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/8016/2/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: Line 296: ASSERT_OK(WaitForNumTabletsOnTS(ts0, kNumTablets, kDefaultTimeout, &tablets, true)); > I understand, but I don't see any code here to delete or otherwise fail tab Without this the tablet copies were failing due to the tablets being in initializing state. -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Hello Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7985 to look at the new patch set (#7). Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. KUDU-2124. Don't hold session lock while initializing a TabletCopySession This patch replaces the interior mutex in TabletCopySourceSession with a dynamic once, so that initialization can happen concurrently. This allows initialization to happen outside of the critical section in the tablet copy service. Implemented a regression test that injects latency into BeginTabletCopySession() calls. Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/tablet_copy_service-test.cc M src/kudu/tserver/tablet_copy_service.cc M src/kudu/tserver/tablet_copy_source_session.cc M src/kudu/tserver/tablet_copy_source_session.h M src/kudu/tserver/tablet_server-test-base.h M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_server_test_util.cc M src/kudu/tserver/tablet_server_test_util.h 11 files changed, 243 insertions(+), 62 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/7985/7 -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Adar Dembo has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-2141. master: Remove DCHECK when tablet report has no opid index
Mike Percy has uploaded a new change for review. http://gerrit.cloudera.org:8080/8019 Change subject: KUDU-2141. master: Remove DCHECK when tablet report has no opid_index .. KUDU-2141. master: Remove DCHECK when tablet report has no opid_index Commit 2108767bf5331e0f3beccd56a987cb413cca380a made it possible for a tombstoned replica to report an empty committed config with no opid_index associated with it. The master should handle such a tablet by simply ignoring it. The included modification to tombstoned_voting-itest.cc causes the test to fail without the included changes to catalog_manager.cc Change-Id: Ia99936b11b49a4bd70dbc065f2d16136eb5b8bda --- M src/kudu/integration-tests/tombstoned_voting-itest.cc M src/kudu/master/catalog_manager.cc 2 files changed, 31 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/8019/1 -- To view, visit http://gerrit.cloudera.org:8080/8019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia99936b11b49a4bd70dbc065f2d16136eb5b8bda Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy
[kudu-CR] [tests] de-flaking catalog manager tsk-itest
Andrew Wong has posted comments on this change. Change subject: [tests] de-flaking catalog_manager_tsk-itest .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/8017/1/src/kudu/integration-tests/catalog_manager_tsk-itest.cc File src/kudu/integration-tests/catalog_manager_tsk-itest.cc: PS1, Line 84: // Add master-only flags. Someone newly reading through this test might not understand why all these flags are necessary without reading the commit msg. Could you comment with a high-level statement explaining what the desired behavior of the master is? PS1, Line 98: // Add tserver-only flags. Same here. -- To view, visit http://gerrit.cloudera.org:8080/8017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I50cee27a579cffa7232137c7039b02a1ad4ab7eb Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-2141. master: Remove DCHECK when tablet report has no opid index
Adar Dembo has posted comments on this change. Change subject: KUDU-2141. master: Remove DCHECK when tablet report has no opid_index .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99936b11b49a4bd70dbc065f2d16136eb5b8bda Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-2141. master: Remove DCHECK when tablet report has no opid index
Andrew Wong has posted comments on this change. Change subject: KUDU-2141. master: Remove DCHECK when tablet report has no opid_index .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/8019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99936b11b49a4bd70dbc065f2d16136eb5b8bda Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-2141. master: Remove DCHECK when tablet report has no opid index
Mike Percy has submitted this change and it was merged. Change subject: KUDU-2141. master: Remove DCHECK when tablet report has no opid_index .. KUDU-2141. master: Remove DCHECK when tablet report has no opid_index Commit 2108767bf5331e0f3beccd56a987cb413cca380a made it possible for a tombstoned replica to report an empty committed config with no opid_index associated with it. The master should handle such a tablet by simply ignoring it. The included modification to tombstoned_voting-itest.cc causes the test to fail without the included changes to catalog_manager.cc Change-Id: Ia99936b11b49a4bd70dbc065f2d16136eb5b8bda Reviewed-on: http://gerrit.cloudera.org:8080/8019 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M src/kudu/integration-tests/tombstoned_voting-itest.cc M src/kudu/master/catalog_manager.cc 2 files changed, 31 insertions(+), 10 deletions(-) Approvals: Andrew Wong: Looks good to me, but someone else must approve Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia99936b11b49a4bd70dbc065f2d16136eb5b8bda Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
